* Re: [PATCH 10/20] gpio: pca953x: Add optional reset gpio control
From: Lothar Waßmann @ 2016-12-30 7:06 UTC (permalink / raw)
To: Steve Longerbeam
Cc: mark.rutland, gnurou, devel, p.zabel, devicetree, gregkh,
linus.walleij, linux, linux-kernel, Steve Longerbeam, robh+dt,
kernel, linux-gpio, fabio.estevam, mchehab, shawnguo,
linux-arm-kernel, linux-media
In-Reply-To: <1483050455-10683-11-git-send-email-steve_longerbeam@mentor.com>
Hi,
On Thu, 29 Dec 2016 14:27:25 -0800 Steve Longerbeam wrote:
> Add optional reset-gpios pin control. If present, de-assert the
> specified reset gpio pin to bring the chip out of reset.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> ---
>
> v2:
> - documented optional reset-gpios property in
> Documentation/devicetree/bindings/gpio/gpio-pca953x.txt.
> ---
> Documentation/devicetree/bindings/gpio/gpio-pca953x.txt | 4 ++++
> drivers/gpio/gpio-pca953x.c | 17 +++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> index 08dd15f..da54f4c 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> @@ -29,6 +29,10 @@ Required properties:
> onsemi,pca9654
> exar,xra1202
>
> +Optional properties:
> + - reset-gpios: GPIO specification for the RESET input
> +
> +
> Example:
>
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index d5d72d8..d1c0bd5 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -22,6 +22,7 @@
> #include <linux/of_platform.h>
> #include <linux/acpi.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/gpio/consumer.h>
>
> #define PCA953X_INPUT 0
> #define PCA953X_OUTPUT 1
> @@ -133,6 +134,7 @@ struct pca953x_chip {
> const char *const *names;
> unsigned long driver_data;
> struct regulator *regulator;
> + struct gpio_desc *reset_gpio;
>
> const struct pca953x_reg_config *regs;
>
> @@ -756,6 +758,21 @@ static int pca953x_probe(struct i2c_client *client,
> } else {
> chip->gpio_start = -1;
> irq_base = 0;
> +
> + /* see if we need to de-assert a reset pin */
> + chip->reset_gpio = devm_gpiod_get_optional(&client->dev,
> + "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(chip->reset_gpio)) {
> + dev_err(&client->dev, "request for reset pin failed\n");
> + return PTR_ERR(chip->reset_gpio);
> + }
> +
> + if (chip->reset_gpio) {
> + /* bring chip out of reset */
> + dev_info(&client->dev, "releasing reset\n");
> + gpiod_set_value(chip->reset_gpio, 0);
>
The pin is already initialized to the inactive state thru the
GPIOD_OUT_LOW flag in devm_gpiod_get_optional(), so this call to
gpiod_set_value() is useless.
Lothar Waßmann
_______________________________________________
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] ARM: dts: vf610-zii-dev-rev-b: Add missing newline
From: Shawn Guo @ 2016-12-30 7:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andreas Färber,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
David S . Miller, Sascha Hauer, Stefan Agner, Rob Herring,
Mark Rutland, Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161127213516.GF13318-g2DYL2Zd6BY@public.gmane.org>
On Sun, Nov 27, 2016 at 10:35:16PM +0100, Andrew Lunn wrote:
> On Sun, Nov 27, 2016 at 10:30:54PM +0100, Andreas Färber wrote:
> > Hi,
> >
> > Am 27.11.2016 um 22:17 schrieb Andrew Lunn:
> > > On Sun, Nov 27, 2016 at 08:54:44PM +0100, Andreas Färber wrote:
> > >> Found while reviewing Marvell dsa bindings usage.
> > >
> > > Hi Andreas
> > >
> > > It is good practice to put the maintainer you expect to accept the
> > > patch on the To: line. You have at least two different maintainers on
> > > Cc: so it is currently ambiguous. And these lists can be high volume,
> > > so without a copy in the maintainers inbox, patches can be overlooked.
> >
> > As a vf610 DT patch with LAKML in To I am expecting it to be handled by
> > Shawn or anyone from the NXP/ARM side.
>
> So please have Shawn as To:
>
> The patch you are fixing went through Dave Miller, who is also on Cc:,
> hence the ambiguity.
The good practice was broken from the beginning when patch "arm: vf610:
zii devel b: Add support for switch interrupts") went through net tree,
even without me on copy.
Shawn
--
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] ARM: dts: vf610-zii-dev-rev-b: Add missing newline
From: Shawn Guo @ 2016-12-30 7:10 UTC (permalink / raw)
To: Andreas Färber
Cc: linux-arm-kernel, Andrew Lunn, David S . Miller, Sascha Hauer,
Stefan Agner, Rob Herring, Mark Rutland, Russell King, devicetree,
linux-kernel
In-Reply-To: <1480276484-5482-1-git-send-email-afaerber@suse.de>
On Sun, Nov 27, 2016 at 08:54:44PM +0100, Andreas Färber wrote:
> Found while reviewing Marvell dsa bindings usage.
>
> Fixes: f283745b3caf ("arm: vf610: zii devel b: Add support for switch interrupts")
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
Applied, thanks.
^ permalink raw reply
* [PATCH v3] net: ethernet: faraday: To support device tree usage.
From: Greentime Hu @ 2016-12-30 7:37 UTC (permalink / raw)
To: netdev, devicetree, andrew, arnd, linux-kernel, jiri; +Cc: Greentime Hu
Signed-off-by: Greentime Hu <green.hu@gmail.com>
---
Changes in v3:
- Nothing changed in this patch but I have committed andestech to vendor-prefixes.txt.
drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index dce5f7b..5d70ee9 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id ftmac100_of_ids[] = {
+ { .compatible = "andestech,atmac100" },
+ { }
+};
+
static struct platform_driver ftmac100_driver = {
.probe = ftmac100_probe,
.remove = __exit_p(ftmac100_remove),
.driver = {
.name = DRV_NAME,
+ .of_match_table = ftmac100_of_ids
},
};
@@ -1200,3 +1206,4 @@ static void __exit ftmac100_exit(void)
MODULE_AUTHOR("Po-Yu Chuang <ratbert@faraday-tech.com>");
MODULE_DESCRIPTION("FTMAC100 driver");
MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, ftmac100_of_ids);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] pinctrl: fix DT bindings for marvell,kirkwood-pinctrl
From: Linus Walleij @ 2016-12-30 7:37 UTC (permalink / raw)
To: Andreas Klinger
Cc: devicetree@vger.kernel.org, Rob Herring, Mark Rutland,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20161213230827.GA5286@andreas>
On Wed, Dec 14, 2016 at 12:08 AM, Andreas Klinger <ak@it-klinger.de> wrote:
> 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>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 3/3] power: supply: bq24735-charger: allow chargers to share the ac-detect gpio
From: Linus Walleij @ 2016-12-30 7:49 UTC (permalink / raw)
To: Peter Rosin
Cc: Sebastian Reichel, Alexandre Courbot,
linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
devicetree@vger.kernel.org, linux-gpio@vger.kernel.org
In-Reply-To: <e1667d9d-17bf-2fe5-8a65-af73b6507cbd@axentia.se>
On Wed, Dec 14, 2016 at 6:41 PM, Peter Rosin <peda@axentia.se> wrote:
> On 2016-12-14 18:01, Sebastian Reichel wrote:
>> [of course I forgot to actually add gpio people, let's try again]
>>
>> On Wed, Dec 14, 2016 at 05:59:21PM +0100, Sebastian Reichel wrote:
>>> Hi,
>>>
>>> On Wed, Dec 14, 2016 at 12:56:45AM +0100, Peter Rosin wrote:
>>>> 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.
>>>
>>> Adding GPIO subsystem people to see if they can come up with
>>> something in the gpiod API for this usecase.
>
> Right, I don't like how my new code steps away from gpio descriptors.
The issue of shared gpiods have come up over and over again.
For example the messy regulator code needs this too.
It is better if we implement something like gpiod_get_shared()
in the gpiolib of these cases.
Just put a refcount in struct gpio_desc in drivers/gpio/gpiolib.h
for this case I guess?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 4/6] bus: add driver for the Technologic Systems NBUS
From: Linus Walleij @ 2016-12-30 7:58 UTC (permalink / raw)
To: Sebastien Bourdelin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel,
mark-L1vi/lXTdtvnC/t2CciAbw, kris-L1vi/lXTdtvnC/t2CciAbw,
Simon Horman, Thierry Reding, Jon Hunter, Florian Fainelli,
Fabio Estevam, Sascha Hauer, Shawn Guo, Russell King,
Guenter Roeck, Wim Van Sebroeck, Mark Rutland, Rob Herring
In-Reply-To: <20161214231237.17496-5-sebastien.bourdelin-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
On Thu, Dec 15, 2016 at 12:12 AM, Sebastien Bourdelin
<sebastien.bourdelin-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org> wrote:
> This driver implements a GPIOs bit-banged bus, called the NBUS by
> Technologic Systems. It is used to communicate with the peripherals in
> the FPGA on the TS-4600 SoM.
>
> Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
(...)
> +#include <linux/gpio.h>
Use:
#include <linux/gpio/consumer.h>
instead, and deal with the fallout.
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
Don't use this.
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +static DEFINE_MUTEX(ts_nbus_lock);
> +static bool ts_nbus_ready;
Why not move this to the struct ts_nbus state container?
It seems to be per-bus not per-system.
> +#define TS_NBUS_READ_MODE 0
> +#define TS_NBUS_WRITE_MODE 1
> +#define TS_NBUS_DIRECTION_IN 0
> +#define TS_NBUS_DIRECTION_OUT 1
> +#define TS_NBUS_WRITE_ADR 0
> +#define TS_NBUS_WRITE_VAL 1
> +
> +struct ts_nbus {
> + struct pwm_device *pwm;
> + int num_data;
> + int *data;
> + int csn;
> + int txrx;
> + int strobe;
> + int ale;
> + int rdy;
> +};
> +
> +static struct ts_nbus *ts_nbus;
Nopes. No singletons please.
Use the state container pattern:
Documentation/driver-model/design-patterns.txt
> +/*
> + * request all gpios required by the bus.
> + */
> +static int ts_nbus_init(struct platform_device *pdev)
> +{
> + int err;
> + int i;
> +
> + for (i = 0; i < ts_nbus->num_data; i++) {
> + err = devm_gpio_request_one(&pdev->dev, ts_nbus->data[i],
> + GPIOF_OUT_INIT_HIGH,
> + "TS NBUS data");
> + if (err)
> + return err;
> + }
DO not use the legacy GPIO API anywhere.
Reference the device and use gpiod_get() simple, fair and square.
Store struct gpio_desc * pointers in your state container instead.
> +static int ts_nbus_get_of_pdata(struct device *dev, struct device_node *np)
> +{
> + int num_data;
> + int *data;
> + int ret;
> + int i;
> +
> + ret = of_gpio_named_count(np, "data-gpios");
> + if (ret < 0) {
> + dev_err(dev,
> + "failed to count GPIOs in DT property data-gpios\n");
> + return ret;
> + }
Do not reinvent the wheel.
Use devm_gpiod_get_array().
> + ret = of_get_named_gpio(np, "csn-gpios", 0);
And again use devm_gpiod_get(), and gpiod_* accessors.
Applies everywhere.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] Documentation: simple-card: add full path to widgets.txt
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2016-12-30 8:00 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
broonie-DgEjT+Ai2ygdnm+yROfE0A, Yegor Yefremov
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/sound/simple-card.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index c7a9393..4198fe5 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -10,7 +10,8 @@ Optional properties:
- simple-audio-card,name : User specified audio sound card name, one string
property.
-- simple-audio-card,widgets : Please refer to widgets.txt.
+- simple-audio-card,widgets : Please refer to
+ Documentation/devicetree/bindings/sound/widgets.txt.
- simple-audio-card,routing : A list of the connections between audio components.
Each entry is a pair of strings, the first being the
connection's sink, the second being the connection's
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 5/6] ARM: dts: TS-4600: add NBUS support
From: Linus Walleij @ 2016-12-30 8:01 UTC (permalink / raw)
To: Sebastien Bourdelin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel,
mark-L1vi/lXTdtvnC/t2CciAbw, kris-L1vi/lXTdtvnC/t2CciAbw,
Simon Horman, Thierry Reding, Jon Hunter, Florian Fainelli,
Fabio Estevam, Sascha Hauer, Shawn Guo, Russell King,
Guenter Roeck, Wim Van Sebroeck, Mark Rutland, Rob Herring
In-Reply-To: <20161214231237.17496-6-sebastien.bourdelin-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
On Thu, Dec 15, 2016 at 12:12 AM, Sebastien Bourdelin
<sebastien.bourdelin-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org> wrote:
> This commit enables the NBUS on the TS-4600, using the ts-nbus driver.
>
> Signed-off-by: Sebastien Bourdelin <sebastien.bourdelin-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org>
> + nbus {
> + compatible = "technologic,ts-nbus", "simple-bus";
> + pinctrl-0 = <&nbus_pins>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pwms = <&pwm 2 83>;
> + data-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH
> + &gpio0 1 GPIO_ACTIVE_HIGH
> + &gpio0 2 GPIO_ACTIVE_HIGH
> + &gpio0 3 GPIO_ACTIVE_HIGH
> + &gpio0 4 GPIO_ACTIVE_HIGH
> + &gpio0 5 GPIO_ACTIVE_HIGH
> + &gpio0 6 GPIO_ACTIVE_HIGH
> + &gpio0 7 GPIO_ACTIVE_HIGH>;
> + csn-gpios = <&gpio0 16 GPIO_ACTIVE_HIGH>;
> + txrx-gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
> + strobe-gpios = <&gpio0 25 GPIO_ACTIVE_HIGH>;
> + ale-gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>;
> + rdy-gpios = <&gpio0 21 GPIO_ACTIVE_HIGH>;
devm_gpiod_get(&pdev->dev, "csn", GPIOD_OUT_HIGH); etc will get these
from the spawned
platform device.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] Documentation: panel-dpi: fix path to display-timing.txt
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2016-12-30 8:05 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, airlied-cv59FeDIM0c,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Yegor Yefremov
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/display/panel/panel-dpi.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.txt b/Documentation/devicetree/bindings/display/panel/panel-dpi.txt
index b52ac52..d4add13 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-dpi.txt
+++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.txt
@@ -12,7 +12,7 @@ Optional properties:
Required nodes:
- "panel-timing" containing video timings
- (Documentation/devicetree/bindings/display/display-timing.txt)
+ (Documentation/devicetree/bindings/display/panel/display-timing.txt)
- Video port for DPI input
Example
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 4/4] mmc: pwrseq-simple: add disable-post-power-on option
From: Linus Walleij @ 2016-12-30 8:05 UTC (permalink / raw)
To: Matt Ranostay
Cc: Ulf Hansson, Rob Herring, devicetree@vger.kernel.org,
linux-mmc@vger.kernel.org, Tony Lindgren, Liam Breck
In-Reply-To: <CAJ_EiSRAQefYj+B3AKLb1EWXQUGccV=Z4op0T5sAdo3-vDVOhg@mail.gmail.com>
On Mon, Dec 19, 2016 at 1:01 AM, Matt Ranostay <matt@ranostay.consulting> wrote:
> * SD8787 has a "powerdown" line, and CW1200 has a "powerup" line.. I
> know this is a simple logic inversion.
If this is a GPIO line, the GPIO subsystem can flag a line for
inverted logic. GPIO_ACTIVE_LOW from device tree for example.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] ARM: dts: am572x-idk: Add gpios property to control PCIE_RESETn
From: Kishon Vijay Abraham I @ 2016-12-30 8:07 UTC (permalink / raw)
To: Benoît Cousson, Tony Lindgren, linux-omap, devicetree,
linux-arm-kernel, linux-kernel
Cc: Rob Herring, Mark Rutland, Russell King, nsekhar, kishon
Add 'gpios' property to pcie1 dt node and populate it with
GPIO3_23 in order to drive PCIE_RESETn high.
This gets PCIe cards to be detected in AM572X IDK board.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
arch/arm/boot/dts/am572x-idk.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/am572x-idk.dts b/arch/arm/boot/dts/am572x-idk.dts
index 27d9149..1540f7a 100644
--- a/arch/arm/boot/dts/am572x-idk.dts
+++ b/arch/arm/boot/dts/am572x-idk.dts
@@ -87,3 +87,7 @@
&sn65hvs882 {
load-gpios = <&gpio3 19 GPIO_ACTIVE_LOW>;
};
+
+&pcie1 {
+ gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>;
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] gpio: of: Add support for multiple GPIOs in a single GPIO hog
From: Linus Walleij @ 2016-12-30 8:28 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Alexandre Courbot, Rob Herring, Mark Rutland,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1482171694-18237-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
On Mon, Dec 19, 2016 at 7:21 PM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> When listing multiple GPIOs in the "gpios" property of a GPIO hog, only
> the first GPIO is affected. The user is left clueless about the
> disfunctioning of the other GPIOs specified.
>
> Fix this by adding and documenting support for specifying multiple
> GPIOs in a single GPIO hog.
>
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Makes a lot of sense.
Patch applied!
Yours,
Linus Walleij
--
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 3/5] pinctrl: mvebu: pinctrl driver for 98DX3236 SoC
From: Linus Walleij @ 2016-12-30 9:03 UTC (permalink / raw)
To: Chris Packham, Sebastian Hesselbarth, Thomas Petazzoni
Cc: linux-arm-kernel@lists.infradead.org, Kalyan Kinthada,
Rob Herring, Mark Rutland, Laxman Dewangan,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20161222041328.3303-4-chris.packham@alliedtelesis.co.nz>
On Thu, Dec 22, 2016 at 5:13 AM, Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
> From: Kalyan Kinthada <kalyan.kinthada@alliedtelesis.co.nz>
>
> This pinctrl driver supports the 98DX3236, 98DX3336 and 98DX4251 SoCs
> from Marvell.
>
> Signed-off-by: Kalyan Kinthada <kalyan.kinthada@alliedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Looks good to me, Sebastian and/or Thomas P can you ACK this patch?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: Linus Walleij @ 2016-12-30 9:07 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: Uwe Kleine-König, Wolfram Sang, Rob Herring, Maxime Coquelin,
Alexandre Torgue, Patrice Chotard, Russell King,
linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAOAejn1zHMFNiGc-wVbbkC1-JFzG3sjnnUJogtCp9EeAkWsV5A@mail.gmail.com>
On Fri, Dec 23, 2016 at 2:09 PM, M'boumba Cedric Madianga
<cedric.madianga@gmail.com> wrote:
> 2016-12-22 20:11 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
>> On Thu, Dec 22, 2016 at 02:35:02PM +0100, M'boumba Cedric Madianga wrote:
>>> @@ -337,6 +350,16 @@
>>> slew-rate = <2>;
>>> };
>>> };
>>> +
>>> + i2c1_pins_b: i2c1@0 {
>>> + pins1 {
>>> + pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>;
>>> + drive-open-drain;
>>> + };
>>> + pins2 {
>>> + pinmux = <STM32F429_PB6_FUNC_I2C1_SCL>;
>>> + };
>>
>> the second doesn't need the open-drain property? Why?
>
> I thought that open-drain was only needed for SDA line.
> But after double-checking I2C specification, it seems that SDA and SCL
> lines need open-drain or open-collector to perform the wired-AND
> function.
I think I2C SDA/SCL must be open drain by definition.
It also requires pull-up resistors, I guess you have these mounted on the board
so you do not need pull-up from the pin controller?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 7/9] pinctrl: samsung: Add property to mark pad state as suitable for power down
From: Linus Walleij @ 2016-12-30 9:23 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Marek Szyprowski, linux-gpio@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
linux-samsung-soc, Sylwester Nawrocki, Tomasz Figa, Ulf Hansson,
Bartlomiej Zolnierkiewicz, devicetree@vger.kernel.org
In-Reply-To: <20161227153331.jbh7ei6oh3obmnri@kozik-lap>
On Tue, Dec 27, 2016 at 4:33 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Tue, Dec 27, 2016 at 11:30:57AM +0100, Marek Szyprowski wrote:
>> Separate sleep state is needed because of the pin control bindings and
>> design.
>>
>> A separate sleep state is required to let pin control client driver (in this
>> case
>> Exynos I2S driver) let to choose when it is okay to switch pads into sleep
>> state and I see no other way to achieve this.
>
> Maybe the pinctrl API should be extended for this? Doing this in DTS
> just for purpose of passing information between drivers (consumer and
> provider) looks odd.
I don't understand what is going on but you are all smart people so I
guess you're right :)
Tell me if I can help clarifying something...
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 3/6] Documentation: DT: Add bmi160 imu binding
From: Jonathan Cameron @ 2016-12-30 10:40 UTC (permalink / raw)
To: Rob Herring, Marcin Niestroj
Cc: Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Mark Rutland,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161212171500.7w67evyvicdpfkfc@rob-hp-laptop>
On 12/12/16 17:15, Rob Herring wrote:
> On Thu, Dec 08, 2016 at 03:22:56PM +0100, Marcin Niestroj wrote:
>> This adds documentation for Bosch BMI160 Inertial Measurement Unit
>> device-tree bindings.
>>
>> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
>> ---
>> Changes v1 -> v2 (suggested by Rob):
>> * remove "gpio" keyword from interrupts property description
>> * describe "INT1" and "INT2" cases for interrupt-names property
>>
>> .../devicetree/bindings/iio/imu/bmi160.txt | 36 ++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Applied to the togreg branch of iio.git - initially pushed out as testing for
the autobuilders to completely ignore this patch.
Thanks,
Jonathan
>
>>
>> diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>> new file mode 100644
>> index 0000000..ae0112c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>> @@ -0,0 +1,36 @@
>> +Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
>> +and externally connectable Magnetometer
>> +
>> +https://www.bosch-sensortec.com/bst/products/all_products/bmi160
>> +
>> +Required properties:
>> + - compatible : should be "bosch,bmi160"
>> + - reg : the I2C address or SPI chip select number of the sensor
>> + - spi-max-frequency : set maximum clock frequency (only for SPI)
>> +
>> +Optional properties:
>> + - interrupt-parent : should be the phandle of the interrupt controller
>> + - interrupts : interrupt mapping for IRQ, must be IRQ_TYPE_LEVEL_LOW
>> + - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
>> + input, set to "INT2" if INT2 pin should be used instead
>> +
>> +Examples:
>> +
>> +bmi160@68 {
>> + compatible = "bosch,bmi160";
>> + reg = <0x68>;
>> +
>> + interrupt-parent = <&gpio4>;
>> + interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
>> + interrupt-names = "INT1";
>> +};
>> +
>> +bmi160@0 {
>> + compatible = "bosch,bmi160";
>> + reg = <0>;
>> + spi-max-frequency = <10000000>;
>> +
>> + interrupt-parent = <&gpio2>;
>> + interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
>> + interrupt-names = "INT2";
>> +};
>> --
>> 2.10.2
>>
^ permalink raw reply
* [PATCH] bus: imx-weim: Place 'fsl,weim-cs-timing' under the required properties section
From: Fabio Estevam @ 2016-12-30 10:46 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam
From: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
Property 'fsl,weim-cs-timing' is a required one, so place it under
"Required properties" section.
Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
---
Documentation/devicetree/bindings/bus/imx-weim.txt | 27 ++++++++++------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
index 6630d84..a9a125b 100644
--- a/Documentation/devicetree/bindings/bus/imx-weim.txt
+++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
@@ -23,6 +23,18 @@ Required properties:
integer values for each chip-select line in use:
<cs-number> 0 <physical address of mapping> <size>
+ - fsl,weim-cs-timing: The timing array, contains timing values for the
+ child node. We can get the CS index from the child
+ node's "reg" property. The number of registers depends
+ on the selected chip.
+ For i.MX1, i.MX21 ("fsl,imx1-weim") there are two
+ registers: CSxU, CSxL.
+ For i.MX25, i.MX27, i.MX31 and i.MX35 ("fsl,imx27-weim")
+ there are three registers: CSCRxU, CSCRxL, CSCRxA.
+ For i.MX50, i.MX53 ("fsl,imx50-weim"),
+ i.MX51 ("fsl,imx51-weim") and i.MX6Q ("fsl,imx6q-weim")
+ there are six registers: CSxGCR1, CSxGCR2, CSxRCR1,
+ CSxRCR2, CSxWCR1, CSxWCR2.
Optional properties:
@@ -44,21 +56,6 @@ Optional properties:
what bootloader sets up in IOMUXC_GPR1[11:0] will be
used.
-Timing property for child nodes. It is mandatory, not optional.
-
- - fsl,weim-cs-timing: The timing array, contains timing values for the
- child node. We can get the CS index from the child
- node's "reg" property. The number of registers depends
- on the selected chip.
- For i.MX1, i.MX21 ("fsl,imx1-weim") there are two
- registers: CSxU, CSxL.
- For i.MX25, i.MX27, i.MX31 and i.MX35 ("fsl,imx27-weim")
- there are three registers: CSCRxU, CSCRxL, CSCRxA.
- For i.MX50, i.MX53 ("fsl,imx50-weim"),
- i.MX51 ("fsl,imx51-weim") and i.MX6Q ("fsl,imx6q-weim")
- there are six registers: CSxGCR1, CSxGCR2, CSxRCR1,
- CSxRCR2, CSxWCR1, CSxWCR2.
-
Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
weim: weim@021b8000 {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 4/6] iio: bmi160: Protect data transmission with mutex
From: Jonathan Cameron @ 2016-12-30 10:47 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161208142259.26230-5-m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On 08/12/16 14:22, Marcin Niestroj wrote:
> There are currently two possible cases for data transmission with the
> sensor: read/write by iio sysfs and read in trigger interrupt
> handler. Hence we need to protect data transmission using regmap_*
> functions with mutex.
Except that the regmap functions themselves have build in protection against
any such clashes. Hence I don't 'think' this is necessary as we don't
have any open coded read / update loops in here that I can see.
Jonathan
>
> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
> ---
> Patch introduced in v2
>
> drivers/iio/imu/bmi160/bmi160_core.c | 39 +++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> index e0251b8..095533c 100644
> --- a/drivers/iio/imu/bmi160/bmi160_core.c
> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> @@ -2,6 +2,7 @@
> * BMI160 - Bosch IMU (accel, gyro plus external magnetometer)
> *
> * Copyright (c) 2016, Intel Corporation.
> + * Copyright (c) 2016, Grinn
> *
> * This file is subject to the terms and conditions of version 2 of
> * the GNU General Public License. See the file COPYING in the main
> @@ -112,6 +113,7 @@ enum bmi160_sensor_type {
>
> struct bmi160_data {
> struct regmap *regmap;
> + struct mutex mutex;
> };
>
> const struct regmap_config bmi160_regmap_config = {
> @@ -285,7 +287,9 @@ int bmi160_set_mode(struct bmi160_data *data, enum bmi160_sensor_type t,
> else
> cmd = bmi160_regs[t].pmu_cmd_suspend;
>
> + mutex_lock(&data->mutex);
> ret = regmap_write(data->regmap, BMI160_REG_CMD, cmd);
> + mutex_unlock(&data->mutex);
> if (ret < 0)
> return ret;
>
> @@ -298,6 +302,7 @@ static
> int bmi160_set_scale(struct bmi160_data *data, enum bmi160_sensor_type t,
> int uscale)
> {
> + int ret;
> int i;
>
> for (i = 0; i < bmi160_scale_table[t].num; i++)
> @@ -307,8 +312,12 @@ int bmi160_set_scale(struct bmi160_data *data, enum bmi160_sensor_type t,
> if (i == bmi160_scale_table[t].num)
> return -EINVAL;
>
> - return regmap_write(data->regmap, bmi160_regs[t].range,
> - bmi160_scale_table[t].tbl[i].bits);
> + mutex_lock(&data->mutex);
> + ret = regmap_write(data->regmap, bmi160_regs[t].range,
> + bmi160_scale_table[t].tbl[i].bits);
> + mutex_unlock(&data->mutex);
> +
> + return ret;
> }
>
> static
> @@ -317,7 +326,9 @@ int bmi160_get_scale(struct bmi160_data *data, enum bmi160_sensor_type t,
> {
> int i, ret, val;
>
> + mutex_lock(&data->mutex);
> ret = regmap_read(data->regmap, bmi160_regs[t].range, &val);
> + mutex_unlock(&data->mutex);
> if (ret < 0)
> return ret;
>
> @@ -340,7 +351,9 @@ static int bmi160_get_data(struct bmi160_data *data, int chan_type,
>
> reg = bmi160_regs[t].data + (axis - IIO_MOD_X) * sizeof(__le16);
>
> + mutex_lock(&data->mutex);
> ret = regmap_bulk_read(data->regmap, reg, &sample, sizeof(__le16));
> + mutex_unlock(&data->mutex);
> if (ret < 0)
> return ret;
>
> @@ -353,6 +366,7 @@ static
> int bmi160_set_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> int odr, int uodr)
> {
> + int ret;
> int i;
>
> for (i = 0; i < bmi160_odr_table[t].num; i++)
> @@ -363,10 +377,14 @@ int bmi160_set_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> if (i >= bmi160_odr_table[t].num)
> return -EINVAL;
>
> - return regmap_update_bits(data->regmap,
> - bmi160_regs[t].config,
> - bmi160_regs[t].config_odr_mask,
> - bmi160_odr_table[t].tbl[i].bits);
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(data->regmap,
> + bmi160_regs[t].config,
> + bmi160_regs[t].config_odr_mask,
> + bmi160_odr_table[t].tbl[i].bits);
> + mutex_unlock(&data->mutex);
> +
> + return ret;
> }
>
> static int bmi160_get_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> @@ -374,7 +392,9 @@ static int bmi160_get_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> {
> int i, val, ret;
>
> + mutex_lock(&data->mutex);
> ret = regmap_read(data->regmap, bmi160_regs[t].config, &val);
> + mutex_unlock(&data->mutex);
> if (ret < 0)
> return ret;
>
> @@ -402,14 +422,18 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p)
> int i, ret, j = 0, base = BMI160_REG_DATA_MAGN_XOUT_L;
> __le16 sample;
>
> + mutex_lock(&data->mutex);
> for_each_set_bit(i, indio_dev->active_scan_mask,
> indio_dev->masklength) {
> ret = regmap_bulk_read(data->regmap, base + i * sizeof(__le16),
> &sample, sizeof(__le16));
> - if (ret < 0)
> + if (ret < 0) {
> + mutex_unlock(&data->mutex);
> goto done;
> + }
> buf[j++] = sample;
> }
> + mutex_unlock(&data->mutex);
>
> iio_push_to_buffers_with_timestamp(indio_dev, buf,
> iio_get_time_ns(indio_dev));
> @@ -575,6 +599,7 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> data = iio_priv(indio_dev);
> dev_set_drvdata(dev, indio_dev);
> data->regmap = regmap;
> + mutex_init(&data->mutex);
>
> ret = bmi160_chip_init(data, use_spi);
> if (ret < 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 v2 1/6] iio: bmi160: Add of device table for i2c
From: Jonathan Cameron @ 2016-12-30 10:47 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161208142259.26230-2-m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On 08/12/16 14:22, Marcin Niestroj wrote:
> From now on we can add bmi160 device to device-tree by specifying
> compatible string.
>
> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
Other than the fact this should really have been two patches, this looks good.
Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders
to play with it.
Thanks,
Jonathan
> ---
> Patch introduced in v2
>
> drivers/iio/imu/bmi160/bmi160_i2c.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c
> index 07a179d..155a31f 100644
> --- a/drivers/iio/imu/bmi160/bmi160_i2c.c
> +++ b/drivers/iio/imu/bmi160/bmi160_i2c.c
> @@ -11,10 +11,11 @@
> * - 0x68 if SDO is pulled to GND
> * - 0x69 if SDO is pulled to VDDIO
> */
> -#include <linux/module.h>
> +#include <linux/acpi.h>
> #include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/regmap.h>
> -#include <linux/acpi.h>
Whilst I have no objection to reordering headers so they are alphabetical
(as long as not clarity is lost) it should really be in it's own patch...
>
> #include "bmi160.h"
>
> @@ -56,10 +57,19 @@ static const struct acpi_device_id bmi160_acpi_match[] = {
> };
> MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id bmi160_of_match[] = {
> + { .compatible = "bosch,bmi160" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bmi160_of_match);
> +#endif
> +
> static struct i2c_driver bmi160_i2c_driver = {
> .driver = {
> .name = "bmi160_i2c",
> .acpi_match_table = ACPI_PTR(bmi160_acpi_match),
> + .of_match_table = of_match_ptr(bmi160_of_match),
> },
> .probe = bmi160_i2c_probe,
> .remove = bmi160_i2c_remove,
>
--
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 5/6] iio: bmi160: Fix time needed to sleep after command execution
From: Jonathan Cameron @ 2016-12-30 10:49 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161208142259.26230-6-m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On 08/12/16 14:22, Marcin Niestroj wrote:
> Datasheet specifies typical and maximum execution times for which CMD
> register is occupied after previous command execution. We took these
> values as minimum and maximum time for usleep_range() call before making
> a new command execution.
>
> To be sure, that the CMD register is no longer occupied we need to wait
> *at least* the maximum time specified by datasheet.
>
> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
This looks like a definite bug that we would want to fix in stable as well.
If possible, could you ensure this is the first patch in the updated series?
Thanks,
Jonathan
> ---
> Patch introduced in v2
>
> drivers/iio/imu/bmi160/bmi160_core.c | 25 ++++++-------------------
> 1 file changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> index 095533c..88bcf3f 100644
> --- a/drivers/iio/imu/bmi160/bmi160_core.c
> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> @@ -67,10 +67,8 @@
>
> #define BMI160_REG_DUMMY 0x7F
>
> -#define BMI160_ACCEL_PMU_MIN_USLEEP 3200
> -#define BMI160_ACCEL_PMU_MAX_USLEEP 3800
> -#define BMI160_GYRO_PMU_MIN_USLEEP 55000
> -#define BMI160_GYRO_PMU_MAX_USLEEP 80000
> +#define BMI160_ACCEL_PMU_MIN_USLEEP 3800
> +#define BMI160_GYRO_PMU_MIN_USLEEP 80000
> #define BMI160_SOFTRESET_USLEEP 1000
>
> #define BMI160_CHANNEL(_type, _axis, _index) { \
> @@ -153,20 +151,9 @@ static struct bmi160_regs bmi160_regs[] = {
> },
> };
>
> -struct bmi160_pmu_time {
> - unsigned long min;
> - unsigned long max;
> -};
> -
> -static struct bmi160_pmu_time bmi160_pmu_time[] = {
> - [BMI160_ACCEL] = {
> - .min = BMI160_ACCEL_PMU_MIN_USLEEP,
> - .max = BMI160_ACCEL_PMU_MAX_USLEEP
> - },
> - [BMI160_GYRO] = {
> - .min = BMI160_GYRO_PMU_MIN_USLEEP,
> - .max = BMI160_GYRO_PMU_MIN_USLEEP,
> - },
> +static unsigned long bmi160_pmu_time[] = {
> + [BMI160_ACCEL] = BMI160_ACCEL_PMU_MIN_USLEEP,
> + [BMI160_GYRO] = BMI160_GYRO_PMU_MIN_USLEEP,
> };
>
> struct bmi160_scale {
> @@ -293,7 +280,7 @@ int bmi160_set_mode(struct bmi160_data *data, enum bmi160_sensor_type t,
> if (ret < 0)
> return ret;
>
> - usleep_range(bmi160_pmu_time[t].min, bmi160_pmu_time[t].max);
> + usleep_range(bmi160_pmu_time[t], bmi160_pmu_time[t] + 1000);
>
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH v2 5/6] iio: bmi160: Fix time needed to sleep after command execution
From: Jonathan Cameron @ 2016-12-30 10:51 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <7668f191-dafd-e616-171a-cafc52791292-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 30/12/16 10:49, Jonathan Cameron wrote:
> On 08/12/16 14:22, Marcin Niestroj wrote:
>> Datasheet specifies typical and maximum execution times for which CMD
>> register is occupied after previous command execution. We took these
>> values as minimum and maximum time for usleep_range() call before making
>> a new command execution.
>>
>> To be sure, that the CMD register is no longer occupied we need to wait
>> *at least* the maximum time specified by datasheet.
>>
>> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
> This looks like a definite bug that we would want to fix in stable as well.
> If possible, could you ensure this is the first patch in the updated series?
Actually don't worry. It applies cleanly to my fixes-togreg-post-rc1 branch anyway
so I'll take it that way and send upstream asap.
Applied to the fixes-togreg-post-rc1 branch and marked for stable.
Thanks,
Jonathan
>
> Thanks,
>
> Jonathan
>> ---
>> Patch introduced in v2
>>
>> drivers/iio/imu/bmi160/bmi160_core.c | 25 ++++++-------------------
>> 1 file changed, 6 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
>> index 095533c..88bcf3f 100644
>> --- a/drivers/iio/imu/bmi160/bmi160_core.c
>> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
>> @@ -67,10 +67,8 @@
>>
>> #define BMI160_REG_DUMMY 0x7F
>>
>> -#define BMI160_ACCEL_PMU_MIN_USLEEP 3200
>> -#define BMI160_ACCEL_PMU_MAX_USLEEP 3800
>> -#define BMI160_GYRO_PMU_MIN_USLEEP 55000
>> -#define BMI160_GYRO_PMU_MAX_USLEEP 80000
>> +#define BMI160_ACCEL_PMU_MIN_USLEEP 3800
>> +#define BMI160_GYRO_PMU_MIN_USLEEP 80000
>> #define BMI160_SOFTRESET_USLEEP 1000
>>
>> #define BMI160_CHANNEL(_type, _axis, _index) { \
>> @@ -153,20 +151,9 @@ static struct bmi160_regs bmi160_regs[] = {
>> },
>> };
>>
>> -struct bmi160_pmu_time {
>> - unsigned long min;
>> - unsigned long max;
>> -};
>> -
>> -static struct bmi160_pmu_time bmi160_pmu_time[] = {
>> - [BMI160_ACCEL] = {
>> - .min = BMI160_ACCEL_PMU_MIN_USLEEP,
>> - .max = BMI160_ACCEL_PMU_MAX_USLEEP
>> - },
>> - [BMI160_GYRO] = {
>> - .min = BMI160_GYRO_PMU_MIN_USLEEP,
>> - .max = BMI160_GYRO_PMU_MIN_USLEEP,
>> - },
>> +static unsigned long bmi160_pmu_time[] = {
>> + [BMI160_ACCEL] = BMI160_ACCEL_PMU_MIN_USLEEP,
>> + [BMI160_GYRO] = BMI160_GYRO_PMU_MIN_USLEEP,
>> };
>>
>> struct bmi160_scale {
>> @@ -293,7 +280,7 @@ int bmi160_set_mode(struct bmi160_data *data, enum bmi160_sensor_type t,
>> if (ret < 0)
>> return ret;
>>
>> - usleep_range(bmi160_pmu_time[t].min, bmi160_pmu_time[t].max);
>> + usleep_range(bmi160_pmu_time[t], bmi160_pmu_time[t] + 1000);
>>
>> return 0;
>> }
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" 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/3][v2] arm64: freescale: ls2080a: Split devicetree for code resuability
From: Shawn Guo @ 2016-12-30 11:26 UTC (permalink / raw)
To: Abhimanyu Saini
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, robh-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, scott.wood-3arQi8VN3Tc,
stuart.yoder-3arQi8VN3Tc,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Priyanka Jain,
Ashish Kumar
In-Reply-To: <1480325824-14649-3-git-send-email-abhimanyu.saini-3arQi8VN3Tc@public.gmane.org>
On Mon, Nov 28, 2016 at 03:07:03PM +0530, Abhimanyu Saini wrote:
> LS2088A and LS2080A are similar SoCs with a few differences like
> ARM cores etc.
>
> Reorganize the LS2080A device tree to move the common nodes to:
> - fsl-ls2080a-ls2088a.dtsi
> - fsl-ls2080a-ls2088a-rdb.dts
> - fsl-ls2080a-ls2088a-qds.dts
I do not quite like the naming of these files. Will the following one
be better?
- fsl-ls208xa.dtsi
- fsl-ls208xa-rdb.dtsi
- fsl-ls208xa-qds.dtsi
Also if a DTS file is only to be included by another one, not to
generate a DTB, we name it .dtsi rather than .dts.
Shawn
>
> Signed-off-by: Abhimanyu Saini <abhimanyu.saini-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Priyanka Jain <priyanka.jain-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Ashish Kumar <ashish.kumar-3arQi8VN3Tc@public.gmane.org>
> ---
> .../boot/dts/freescale/fsl-ls2080a-ls2088a-qds.dts | 196 ++++++
> .../boot/dts/freescale/fsl-ls2080a-ls2088a-rdb.dts | 152 +++++
> .../boot/dts/freescale/fsl-ls2080a-ls2088a.dtsi | 727 +++++++++++++++++++++
> arch/arm64/boot/dts/freescale/fsl-ls2080a-qds.dts | 154 +----
> arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dts | 110 +---
> arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 706 +-------------------
> 6 files changed, 1100 insertions(+), 945 deletions(-)
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls2080a-ls2088a-qds.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls2080a-ls2088a-rdb.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls2080a-ls2088a.dtsi
--
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 6/6] iio: bmi160: Support hardware fifo
From: Jonathan Cameron @ 2016-12-30 11:29 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen,
Daniel Baluta, Gregor Boirie, Sanchayan Maity, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161208142259.26230-7-m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On 08/12/16 14:22, Marcin Niestroj wrote:
> This patch was developed primarily based on bmc150_accel hardware fifo
> implementation.
>
> IRQ handler was added, which for now is responsible only for handling
> watermark interrupts. The BMI160 chip has two interrupt outputs. By
> default INT is considered to be connected. If INT2 is used instead, the
> interrupt-names device-tree property can be used to specify that.
>
> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
A few bits and bobs inline but basically looking good to me.
Sorry for the delay in getting around to review this! I'd like some input
from Daniel ideally as well, but he is rather busy at the moment so I may
take the final version of this without hearing from him.
thanks,
Jonathan
> ---
> Depends on patch 4 in series
>
> Changes v1 -> v2:
> * add __ prefix to all functions that should be called with mutex held
> (suggested by Peter)
> * get rid of non-constant array size (suggested by Peter)
> * disable irq on init failure and module removal (suggested by Peter)
> * make bmi160_buffer_predisable() the reverse order of the
> bmi160_buffer_postenable() (suggested by Jonathan)
> * remove realignment for iio_info structs (suggested by Jonathan)
>
> drivers/iio/imu/bmi160/bmi160.h | 3 +-
> drivers/iio/imu/bmi160/bmi160_core.c | 618 ++++++++++++++++++++++++++++++++++-
> drivers/iio/imu/bmi160/bmi160_i2c.c | 7 +-
> drivers/iio/imu/bmi160/bmi160_spi.c | 3 +-
> 4 files changed, 613 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
> index d2ae6ed..4a7c10e 100644
> --- a/drivers/iio/imu/bmi160/bmi160.h
> +++ b/drivers/iio/imu/bmi160/bmi160.h
> @@ -4,7 +4,8 @@
> extern const struct regmap_config bmi160_regmap_config;
>
> int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> - const char *name, bool use_spi);
> + const char *name, int irq,
> + bool use_spi, bool block_supported);
> void bmi160_core_remove(struct device *dev);
>
> #endif /* BMI160_H_ */
> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> index 88bcf3f..26404b4 100644
> --- a/drivers/iio/imu/bmi160/bmi160_core.c
> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> @@ -10,7 +10,7 @@
> *
> * IIO core driver for BMI160, with support for I2C/SPI busses
> *
> - * TODO: magnetometer, interrupts, hardware FIFO
> + * TODO: magnetometer, interrupts
> */
> #include <linux/module.h>
> #include <linux/regmap.h>
> @@ -23,8 +23,12 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/sysfs.h>
>
> +#include <linux/of_irq.h>
> +
> #include "bmi160.h"
>
> +#define BMI160_IRQ_NAME "bmi160_event"
Does the device actually support different uses for the two irqs at the
same time? If not I'd be tempted ot just call this bmi160
> +
> #define BMI160_REG_CHIP_ID 0x00
> #define BMI160_CHIP_ID_VAL 0xD1
>
> @@ -35,6 +39,21 @@
> #define BMI160_REG_DATA_GYRO_XOUT_L 0x0C
> #define BMI160_REG_DATA_ACCEL_XOUT_L 0x12
>
> +#define BMI160_REG_STATUS 0x1B
> +#define BMI160_STATUS_MAG_MAN_OP BIT(2)
> +
> +#define BMI160_REG_INT_STATUS0 0x1C
> +
> +#define BMI160_REG_INT_STATUS1 0x1D
> +#define BMI160_INT_STATUS_FWM BIT(6)
> +
> +#define BMI160_REG_INT_STATUS2 0x1E
> +
> +#define BMI160_REG_INT_STATUS3 0x1F
> +
> +#define BMI160_REG_FIFO_LENGTH 0x22
> +#define BMI160_REG_FIFO_DATA 0x24
> +
> #define BMI160_REG_ACCEL_CONFIG 0x40
> #define BMI160_ACCEL_CONFIG_ODR_MASK GENMASK(3, 0)
> #define BMI160_ACCEL_CONFIG_BWP_MASK GENMASK(6, 4)
> @@ -56,6 +75,36 @@
> #define BMI160_GYRO_RANGE_250DPS 0x03
> #define BMI160_GYRO_RANGE_125DPS 0x04
>
> +#define BMI160_REG_FIFO_CONFIG_0 0x46
> +
> +#define BMI160_REG_FIFO_CONFIG_1 0x47
> +#define BMI160_FIFO_GYRO_EN BIT(7)
> +#define BMI160_FIFO_ACCEL_EN BIT(6)
> +#define BMI160_FIFO_MAGN_EN BIT(5)
> +#define BMI160_FIFO_HEADER_EN BIT(4)
> +#define BMI160_FIFO_TAG_INT1_EN BIT(3)
> +#define BMI160_FIFO_TAG_INT2_EN BIT(2)
> +#define BMI160_FIFO_TIME_EN BIT(1)
> +
> +#define BMI160_REG_INT_EN_1 0x51
> +#define BMI160_INT_FWM_EN BIT(6)
> +#define BMI160_INT_FFULL_EN BIT(5)
> +#define BMI160_INT_DRDY_EN BIT(4)
> +
> +#define BMI160_REG_INT_OUT_CTRL 0x53
> +#define BMI160_INT2_OUTPUT_EN BIT(7)
> +#define BMI160_INT1_OUTPUT_EN BIT(3)
> +
> +#define BMI160_REG_INT_LATCH 0x54
> +
> +#define BMI160_REG_INT_MAP_1 0x56
> +#define BMI160_INT1_MAP_DRDY BIT(7)
> +#define BMI160_INT1_MAP_FWM BIT(6)
> +#define BMI160_INT1_MAP_FFULL BIT(5)
> +#define BMI160_INT2_MAP_DRDY BIT(3)
> +#define BMI160_INT2_MAP_FWM BIT(2)
> +#define BMI160_INT2_MAP_FFULL BIT(1)
> +
> #define BMI160_REG_CMD 0x7E
> #define BMI160_CMD_ACCEL_PM_SUSPEND 0x10
> #define BMI160_CMD_ACCEL_PM_NORMAL 0x11
> @@ -67,6 +116,8 @@
>
> #define BMI160_REG_DUMMY 0x7F
>
> +#define BMI160_FIFO_LENGTH 1024
> +
> #define BMI160_ACCEL_PMU_MIN_USLEEP 3800
> #define BMI160_GYRO_PMU_MIN_USLEEP 80000
> #define BMI160_SOFTRESET_USLEEP 1000
> @@ -109,9 +160,34 @@ enum bmi160_sensor_type {
> BMI160_NUM_SENSORS /* must be last */
> };
>
> +struct bmi160_irq_data {
> + unsigned int map_fwm;
> + unsigned int output_en;
> +};
> +
> +static const struct bmi160_irq_data bmi160_irq1_data = {
> + .map_fwm = BMI160_INT1_MAP_FWM,
> + .output_en = BMI160_INT1_OUTPUT_EN,
> +};
> +
> +static const struct bmi160_irq_data bmi160_irq2_data = {
> + .map_fwm = BMI160_INT2_MAP_FWM,
> + .output_en = BMI160_INT2_OUTPUT_EN,
> +};
> +
> struct bmi160_data {
> struct regmap *regmap;
> struct mutex mutex;
> + const struct bmi160_irq_data *irq_data;
> + int irq;
> + bool irq_enabled;
> + int64_t timestamp;
> + int64_t fifo_sample_period;
> + bool fifo_enabled;
> + unsigned int fifo_config;
> + unsigned int fifo_sample_size;
> + u8 *fifo_buffer;
> + unsigned int watermark;
> };
>
> const struct regmap_config bmi160_regmap_config = {
> @@ -374,16 +450,20 @@ int bmi160_set_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> return ret;
> }
>
> -static int bmi160_get_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> - int *odr, int *uodr)
> +static int64_t bmi160_frequency_to_period(int odr, int uodr)
> {
> - int i, val, ret;
> + uint64_t period = 1000000000000000;
> + int64_t frequency = (int64_t) odr * 1000000 + uodr;
>
> - mutex_lock(&data->mutex);
> - ret = regmap_read(data->regmap, bmi160_regs[t].config, &val);
> - mutex_unlock(&data->mutex);
> - if (ret < 0)
> - return ret;
> + do_div(period, frequency);
> +
> + return period;
> +}
> +
> +static const struct bmi160_odr *bmi160_reg_to_odr(enum bmi160_sensor_type t,
> + unsigned int val)
> +{
> + int i;
>
> val &= bmi160_regs[t].config_odr_mask;
>
> @@ -392,10 +472,52 @@ static int bmi160_get_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> break;
>
> if (i >= bmi160_odr_table[t].num)
> - return -EINVAL;
> + return ERR_PTR(-EINVAL);
>
> - *odr = bmi160_odr_table[t].tbl[i].odr;
> - *uodr = bmi160_odr_table[t].tbl[i].uodr;
> + return &bmi160_odr_table[t].tbl[i];
> +}
> +
> +static int __bmi160_get_sample_period(struct bmi160_data *data,
> + enum bmi160_sensor_type t,
> + int64_t *sample_period)
> +{
> + const struct bmi160_odr *odr_entry;
> + int ret;
> + unsigned int val;
> +
> + ret = regmap_read(data->regmap, bmi160_regs[t].config, &val);
> + if (ret < 0)
> + return ret;
> +
> + odr_entry = bmi160_reg_to_odr(t, val);
> + if (IS_ERR(odr_entry))
> + return PTR_ERR(odr_entry);
> +
> + *sample_period = bmi160_frequency_to_period(odr_entry->odr,
> + odr_entry->uodr);
> +
> + return 0;
> +}
> +
> +static int bmi160_get_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
> + int *odr, int *uodr)
> +{
> + const struct bmi160_odr *odr_entry;
> + int ret;
> + unsigned int val;
> +
> + mutex_lock(&data->mutex);
> + ret = regmap_read(data->regmap, bmi160_regs[t].config, &val);
> + mutex_unlock(&data->mutex);
> + if (ret < 0)
> + return ret;
> +
> + odr_entry = bmi160_reg_to_odr(t, val);
> + if (IS_ERR(odr_entry))
> + return PTR_ERR(odr_entry);
> +
> + *odr = odr_entry->odr;
> + *uodr = odr_entry->uodr;
>
> return 0;
> }
> @@ -504,6 +626,356 @@ static const struct attribute_group bmi160_attrs_group = {
> .attrs = bmi160_attrs,
> };
>
> +static int __bmi160_read_sample_period(struct bmi160_data *data,
> + enum bmi160_sensor_type sensor_type)
> +{
> + struct device *dev = regmap_get_device(data->regmap);
> + int64_t uninitialized_var(sample_period);
> + int ret;
> +
> + ret = __bmi160_get_sample_period(data, sensor_type, &sample_period);
> + if (ret < 0)
> + return ret;
> +
> + if (data->fifo_sample_period) {
> + if (data->fifo_sample_period != sample_period) {
Ah, I was wondering how you got around this restriction. A bit more limiting
than having two separate buffers would have been, but easier to handle and
I do wonder how often people actually run these devices with different
sampling frequencies...
> + dev_warn(dev, "Enabled sensors have unequal ODR values\n");
> + return -EINVAL;
> + }
> + } else {
> + data->fifo_sample_period = sample_period;
> + }
> +
> + return 0;
> +}
> +
> +static int __bmi160_fifo_enable(struct iio_dev *indio_dev,
> + struct bmi160_data *data)
> +{
> + struct regmap *regmap = data->regmap;
> + struct device *dev = regmap_get_device(regmap);
> + int ret;
> + int i;
> + unsigned int val;
> + unsigned int fifo_config = 0;
> +
> + /* Set fifo sample size and period */
> + for_each_set_bit(i, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + if (i <= BMI160_SCAN_GYRO_Z)
> + fifo_config |= BMI160_FIFO_GYRO_EN;
> + else if (i <= BMI160_SCAN_ACCEL_Z)
> + fifo_config |= BMI160_FIFO_ACCEL_EN;
> + }
> +
> + data->fifo_sample_period = 0;
> + data->fifo_sample_size = 0;
> + if (fifo_config & BMI160_FIFO_GYRO_EN) {
> + data->fifo_sample_size += 6;
> + ret = __bmi160_read_sample_period(data, BMI160_GYRO);
> + if (ret < 0)
> + return ret;
> + }
> + if (fifo_config & BMI160_FIFO_ACCEL_EN) {
> + data->fifo_sample_size += 6;
> + ret = __bmi160_read_sample_period(data, BMI160_ACCEL);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /*
> + * Set watermark level and write real value back, as it will be used
> + * in timestamp calculation.
> + */
> + val = data->watermark * data->fifo_sample_size;
> + if (val > BMI160_FIFO_LENGTH - 1) {
> + val = BMI160_FIFO_LENGTH - 1;
> + data->watermark = val / data->fifo_sample_size;
> + }
> + val = data->watermark * data->fifo_sample_size / 4;
> +
> + ret = regmap_write(regmap, BMI160_REG_FIFO_CONFIG_0, val);
> + if (ret < 0) {
> + dev_err(dev, "Failed to set watermark\n");
> + return ret;
> + }
> +
> + /* Enable FIFO channels */
> + ret = regmap_write(regmap, BMI160_REG_FIFO_CONFIG_1,
> + fifo_config);
> + if (ret < 0) {
> + dev_err(dev, "Failed to write FIFO_CONFIG_1\n");
> + return ret;
> + }
> +
> + data->fifo_config = fifo_config;
> + data->fifo_enabled = true;
> +
> + return 0;
> +}
> +
> +static int __bmi160_fifo_disable(struct bmi160_data *data)
> +{
> + struct regmap *regmap = data->regmap;
> + struct device *dev = regmap_get_device(regmap);
> + int ret;
> +
> + /* Disable all FIFO channels */
> + ret = regmap_write(regmap, BMI160_REG_FIFO_CONFIG_1, 0);
> + if (ret < 0) {
> + dev_err(dev, "Failed to write FIFO_CONFIG_1\n");
> + return ret;
> + }
> +
> + data->fifo_enabled = false;
> +
> + return 0;
> +}
> +
> +static int bmi160_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct bmi160_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
> + return iio_triggered_buffer_postenable(indio_dev);
> +
> + mutex_lock(&data->mutex);
> + ret = __bmi160_fifo_enable(indio_dev, data);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = regmap_update_bits(data->regmap, BMI160_REG_INT_MAP_1,
> + data->irq_data->map_fwm, data->irq_data->map_fwm);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = regmap_update_bits(data->regmap, BMI160_REG_INT_EN_1,
> + BMI160_INT_FWM_EN, BMI160_INT_FWM_EN);
> +
> +unlock:
> + mutex_unlock(&data->mutex);
> +
> + return ret;
> +}
> +
> +static int bmi160_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct bmi160_data *data = iio_priv(indio_dev);
> + struct regmap *regmap = data->regmap;
> + int ret = 0;
> +
> + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
> + return iio_triggered_buffer_predisable(indio_dev);
> +
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(regmap, BMI160_REG_INT_EN_1,
> + BMI160_INT_FWM_EN, 0);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = regmap_update_bits(data->regmap, BMI160_REG_INT_MAP_1,
> + data->irq_data->map_fwm, 0);
> + if (ret < 0)
> + goto unlock;
> +
> + ret = __bmi160_fifo_disable(data);
> +
> +unlock:
> + mutex_unlock(&data->mutex);
> +
> + return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops bmi160_buffer_ops = {
> + .postenable = bmi160_buffer_postenable,
> + .predisable = bmi160_buffer_predisable,
> +};
> +
> +static ssize_t bmi160_get_fifo_state(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct bmi160_data *data = iio_priv(indio_dev);
> + bool state;
> +
> + mutex_lock(&data->mutex);
> + state = data->fifo_enabled;
> + mutex_unlock(&data->mutex);
> +
> + return sprintf(buf, "%d\n", (int) state);
> +}
> +
> +static ssize_t bmi160_get_fifo_watermark(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct bmi160_data *data = iio_priv(indio_dev);
> + int wm;
> +
> + mutex_lock(&data->mutex);
> + wm = data->watermark;
> + mutex_unlock(&data->mutex);
> +
> + return sprintf(buf, "%d\n", wm);
> +}
> +
> +static IIO_CONST_ATTR(hwfifo_watermark_min, "1");
> +static IIO_CONST_ATTR(hwfifo_watermark_max,
> + __stringify(BMI160_FIFO_LENGTH));
> +static IIO_DEVICE_ATTR(hwfifo_enabled, S_IRUGO,
> + bmi160_get_fifo_state, NULL, 0);
> +static IIO_DEVICE_ATTR(hwfifo_watermark, S_IRUGO,
> + bmi160_get_fifo_watermark, NULL, 0);
> +
> +static const struct attribute *bmi160_fifo_attributes[] = {
> + &iio_const_attr_hwfifo_watermark_min.dev_attr.attr,
> + &iio_const_attr_hwfifo_watermark_max.dev_attr.attr,
> + &iio_dev_attr_hwfifo_watermark.dev_attr.attr,
> + &iio_dev_attr_hwfifo_enabled.dev_attr.attr,
> + NULL,
> +};
> +
> +static int bmi160_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> +{
> + struct bmi160_data *data = iio_priv(indio_dev);
> +
> + if (val > BMI160_FIFO_LENGTH)
> + val = BMI160_FIFO_LENGTH;
> +
> + mutex_lock(&data->mutex);
> + data->watermark = val;
This is a little interesting, in that mostly people might expect a
write to the watermark to be immediately applied if it succeeds.
Perhaps protecting with a claim direct mode call (so it faults out if
the buffer is enabled) would give a more consistent userspace interface.
> + mutex_unlock(&data->mutex);
> +
> + return 0;
> +}
> +
> +static int __bmi160_fifo_transfer(struct bmi160_data *data,
> + char *buffer, int num_bytes)
> +{
> + struct regmap *regmap = data->regmap;
> + struct device *dev = regmap_get_device(regmap);
> + size_t step = regmap_get_raw_read_max(regmap);
> + int ret = 0;
> + int i;
> +
> + if (!step || step > num_bytes)
> + step = num_bytes;
> + else if (step < num_bytes)
> + step = data->fifo_sample_size;
> +
> + for (i = 0; i < num_bytes; i += step) {
> + ret = regmap_raw_read(regmap, BMI160_REG_FIFO_DATA,
> + &buffer[i], step);
When it lands, the new regmap repeated read stuff should simplify this
a little. Looks like it missed the recent merge window..
https://patchwork.ozlabs.org/patch/650718/
It's possible the series has died a death as Leonard has move jobs
and gone rather quiet. If anyone fancies chasing up what is happening with
this and perhaps taking over that work that would be great.
> +
> + if (ret)
> + break;
> + }
> +
> + if (ret)
> + dev_err(dev,
> + "Error transferring data from fifo in single steps of %zu\n",
> + step);
> +
> + return ret;
> +}
> +
> +static int __bmi160_fifo_flush(struct iio_dev *indio_dev,
> + unsigned int samples, bool irq)
> +{
> + struct bmi160_data *data = iio_priv(indio_dev);
> + struct regmap *regmap = data->regmap;
> + struct device *dev = regmap_get_device(regmap);
> + int ret;
> + __le16 fifo_length;
> + unsigned int fifo_samples;
> + unsigned int fifo_bytes;
> + u8 *buffer = data->fifo_buffer;
> + u8 *buffer_iter;
> + int64_t last_timestamp, timestamp;
> + unsigned int last_samples;
> + unsigned int i;
> +
> + /* Get the current FIFO length */
> + ret = regmap_bulk_read(regmap, BMI160_REG_FIFO_LENGTH,
> + &fifo_length, sizeof(__le16));
> + if (ret < 0) {
> + dev_err(dev, "Error reading FIFO_LENGTH\n");
> + return ret;
> + }
> +
> + fifo_bytes = le16_to_cpu(fifo_length);
> + fifo_samples = fifo_bytes / data->fifo_sample_size;
> +
> + if (fifo_bytes % data->fifo_sample_size)
> + dev_warn(dev, "fifo_bytes %u is not dividable by %u\n",
> + fifo_bytes, data->fifo_sample_size);
> +
> + if (!fifo_samples)
> + return 0;
> +
> + if (samples && fifo_samples > samples) {
> + fifo_samples = samples;
> + fifo_bytes = fifo_samples * data->fifo_sample_size;
> + }
> +
> + /*
> + * If we are not called from IRQ, it means that we are flushing data
> + * on demand. In that case we do not have latest timestamp saved in
> + * data->timestamp. Get the time now instead.
> + *
> + * In case of IRQ flush, saved timestamp shows the time when number
> + * of samples configured by watermark were ready. Currently there might
> + * be more samples already.
> + * If we are not called from IRQ, than we are getting the current fifo
> + * length, as we are setting timestamp just after getting it.
> + */
> + if (!irq) {
> + last_timestamp = iio_get_time_ns(indio_dev);
> + last_samples = fifo_samples;
> + } else {
> + last_timestamp = data->timestamp;
> + last_samples = data->watermark;
> + }
> +
> + /* Get all measurements */
> + ret = __bmi160_fifo_transfer(data, buffer, fifo_bytes);
> + if (ret)
> + return ret;
> +
> + /* Handle demux */
> + timestamp = last_timestamp - (last_samples * data->fifo_sample_period);
> + buffer_iter = buffer;
> + for (i = 0; i < fifo_samples; i++) {
> + u8 tmp_buf[32];
> +
> + memcpy(tmp_buf, buffer_iter, data->fifo_sample_size);
> +
> + timestamp += data->fifo_sample_period;
> + iio_push_to_buffers_with_timestamp(indio_dev,
> + tmp_buf,
> + timestamp);
> +
> + buffer_iter += data->fifo_sample_size;
> + }
> +
> + return fifo_samples;
> +}
> +
> +static int bmi160_fifo_flush(struct iio_dev *indio_dev, unsigned int samples)
> +{
> + struct bmi160_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + mutex_lock(&data->mutex);
> + ret = __bmi160_fifo_flush(indio_dev, samples, false);
> + mutex_unlock(&data->mutex);
> +
> + return ret;
> +}
> +
> static const struct iio_info bmi160_info = {
> .driver_module = THIS_MODULE,
> .read_raw = bmi160_read_raw,
> @@ -511,6 +983,15 @@ static const struct iio_info bmi160_info = {
> .attrs = &bmi160_attrs_group,
> };
>
> +static const struct iio_info bmi160_info_fifo = {
> + .driver_module = THIS_MODULE,
> + .read_raw = bmi160_read_raw,
> + .write_raw = bmi160_write_raw,
> + .attrs = &bmi160_attrs_group,
> + .hwfifo_set_watermark = bmi160_set_watermark,
> + .hwfifo_flush_to_buffer = bmi160_fifo_flush,
> +};
> +
> static const char *bmi160_match_acpi_device(struct device *dev)
> {
> const struct acpi_device_id *id;
> @@ -572,12 +1053,75 @@ static void bmi160_chip_uninit(struct bmi160_data *data)
> bmi160_set_mode(data, BMI160_ACCEL, false);
> }
>
> +static int bmi160_enable_irq(struct bmi160_data *data)
> +{
> + int ret;
> +
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(data->regmap, BMI160_REG_INT_OUT_CTRL,
> + data->irq_data->output_en,
> + data->irq_data->output_en);
> + mutex_unlock(&data->mutex);
> +
> + if (ret == 0)
> + data->irq_enabled = true;
> +
> + return ret;
> +}
> +
> +static int bmi160_disable_irq(struct bmi160_data *data)
> +{
> + int ret;
> +
> + if (!data->irq_enabled)
> + return 0;
> +
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(data->regmap, BMI160_REG_INT_OUT_CTRL,
> + data->irq_data->output_en, 0);
> + mutex_unlock(&data->mutex);
> +
> + if (ret == 0)
> + data->irq_enabled = false;
> +
> + return ret;
> +}
> +
> +static irqreturn_t bmi160_irq_thread_handler(int irq, void *p)
> +{
> + struct iio_dev *indio_dev = p;
> + struct bmi160_data *data = iio_priv(indio_dev);
> + struct device *dev = regmap_get_device(data->regmap);
> +
> + mutex_lock(&data->mutex);
> + if (data->fifo_enabled)
> + __bmi160_fifo_flush(indio_dev, BMI160_FIFO_LENGTH, true);
> + else
This is not terribly shared interrupt friendly. We should really be
verifying that the interrupt seen is ours or returning IRQ_NONE otherwise.
Then if we get a load of 'false' interrupts due to hardware issues the
unhandled interrupt stuff in the kernel will deal with it for us.
> + dev_warn(dev,
> + "IRQ has been triggered, but FIFO is not enabled.\n");
> + mutex_unlock(&data->mutex);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t bmi160_irq_handler(int irq, void *p)
> +{
> + struct iio_dev *indio_dev = p;
> + struct bmi160_data *data = iio_priv(indio_dev);
> +
> + data->timestamp = iio_get_time_ns(indio_dev);
> +
> + return IRQ_WAKE_THREAD;
You could use the utility function: iio_pollfunc_store_timestamp
instead of basically replicating it here.
> +}
> +
> int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> - const char *name, bool use_spi)
> + const char *name, int irq,
> + bool use_spi, bool block_supported)
> {
> struct iio_dev *indio_dev;
> struct bmi160_data *data;
> int ret;
> + int irq2;
>
> indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> if (!indio_dev)
> @@ -585,6 +1129,7 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
>
> data = iio_priv(indio_dev);
> dev_set_drvdata(dev, indio_dev);
> + data->irq = irq;
> data->regmap = regmap;
> mutex_init(&data->mutex);
>
> @@ -603,15 +1148,57 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
> indio_dev->info = &bmi160_info;
>
> ret = iio_triggered_buffer_setup(indio_dev, NULL,
> - bmi160_trigger_handler, NULL);
> + bmi160_trigger_handler,
> + &bmi160_buffer_ops);
> if (ret < 0)
> goto uninit;
>
> + if (data->irq > 0) {
> + /* Check which interrupt pin is connected to our board */
> + irq2 = of_irq_get_byname(dev->of_node, "INT2");
> + if (irq2 == data->irq) {
> + dev_dbg(dev, "Using interrupt line INT2\n");
> + data->irq_data = &bmi160_irq2_data;
> + } else {
> + dev_dbg(dev, "Using interrupt line INT1\n");
> + data->irq_data = &bmi160_irq1_data;
> + }
> +
> + ret = devm_request_threaded_irq(dev,
> + data->irq,
> + bmi160_irq_handler,
> + bmi160_irq_thread_handler,
> + IRQF_ONESHOT,
> + BMI160_IRQ_NAME,
> + indio_dev);
> + if (ret)
> + goto buffer_cleanup;
> +
> + ret = bmi160_enable_irq(data);
> + if (ret < 0)
> + goto buffer_cleanup;
> +
> + if (block_supported) {
> + indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
> + indio_dev->info = &bmi160_info_fifo;
> + indio_dev->buffer->attrs = bmi160_fifo_attributes;
> + data->fifo_buffer = devm_kmalloc(dev,
> + BMI160_FIFO_LENGTH,
> + GFP_KERNEL);
> + if (!data->fifo_buffer) {
> + ret = -ENOMEM;
> + goto disable_irq;
> + }
> + }
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret < 0)
> - goto buffer_cleanup;
> + goto disable_irq;
>
> return 0;
> +disable_irq:
> + bmi160_disable_irq(data);
> buffer_cleanup:
> iio_triggered_buffer_cleanup(indio_dev);
> uninit:
> @@ -626,6 +1213,7 @@ void bmi160_core_remove(struct device *dev)
> struct bmi160_data *data = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> + bmi160_disable_irq(data);
> iio_triggered_buffer_cleanup(indio_dev);
> bmi160_chip_uninit(data);
> }
> diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c
> index 155a31f..1a3f4e1 100644
> --- a/drivers/iio/imu/bmi160/bmi160_i2c.c
> +++ b/drivers/iio/imu/bmi160/bmi160_i2c.c
> @@ -24,6 +24,10 @@ static int bmi160_i2c_probe(struct i2c_client *client,
> {
> struct regmap *regmap;
> const char *name = NULL;
> + bool block_supported =
> + i2c_check_functionality(client->adapter, I2C_FUNC_I2C) ||
> + i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_I2C_BLOCK);
>
> regmap = devm_regmap_init_i2c(client, &bmi160_regmap_config);
> if (IS_ERR(regmap)) {
> @@ -35,7 +39,8 @@ static int bmi160_i2c_probe(struct i2c_client *client,
> if (id)
> name = id->name;
>
> - return bmi160_core_probe(&client->dev, regmap, name, false);
> + return bmi160_core_probe(&client->dev, regmap, name, client->irq,
> + false, block_supported);
> }
>
> static int bmi160_i2c_remove(struct i2c_client *client)
> diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
> index d34dfdf..5a53225 100644
> --- a/drivers/iio/imu/bmi160/bmi160_spi.c
> +++ b/drivers/iio/imu/bmi160/bmi160_spi.c
> @@ -26,7 +26,8 @@ static int bmi160_spi_probe(struct spi_device *spi)
> (int)PTR_ERR(regmap));
> return PTR_ERR(regmap);
> }
> - return bmi160_core_probe(&spi->dev, regmap, id->name, true);
> + return bmi160_core_probe(&spi->dev, regmap, id->name, spi->irq,
> + true, true);
> }
>
> static int bmi160_spi_remove(struct spi_device *spi)
>
^ permalink raw reply
* Re: [PATCH 7/9] pinctrl: samsung: Add property to mark pad state as suitable for power down
From: Marek Szyprowski @ 2016-12-30 11:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-gpio, linux-arm-kernel, linux-pm, linux-samsung-soc,
Sylwester Nawrocki, Linus Walleij, Tomasz Figa, Ulf Hansson,
Bartlomiej Zolnierkiewicz, devicetree
In-Reply-To: <20161227153331.jbh7ei6oh3obmnri@kozik-lap>
Hi Krzysztof,
On 2016-12-27 16:33, Krzysztof Kozlowski wrote:
> On Tue, Dec 27, 2016 at 11:30:57AM +0100, Marek Szyprowski wrote:
>> On 2016-12-25 20:19, Krzysztof Kozlowski wrote:
>>> On Fri, Dec 23, 2016 at 01:24:47PM +0100, Marek Szyprowski wrote:
>>>> Add support for special property "samsung,off-state", which indicates a special
>>>> state suitable for device's "sleep" state. Its pin values/properties should
>>>> match the configuration in power down mode. It indicates that pin controller
>>>> can notify runtime power management subsystem, that it is ready for runtime
>>>> suspend if its all pins are configured for such state. This in turn might
>>>> allow to turn respective power domain off to reduce power consumption.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>> Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 8 ++++++++
>>>> drivers/pinctrl/samsung/pinctrl-samsung.c | 4 ++++
>>>> drivers/pinctrl/samsung/pinctrl-samsung.h | 1 +
>>>> 3 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
>>>> index b7bd2e12a269..354eea0e7798 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
>>>> @@ -105,6 +105,7 @@ Required Properties:
>>>> - samsung,pin-drv: Drive strength configuration.
>>>> - samsung,pin-pud-pdn: Pull up/down configuration in power down mode.
>>>> - samsung,pin-drv-pdn: Drive strength configuration in power down mode.
>>>> + - samsung,off-state: Mark this configuration as suitable for bank power off.
>>>> The values specified by these config properties should be derived from the
>>>> hardware manual and these values are programmed as-is into the pin
>>>> @@ -113,6 +114,13 @@ Required Properties:
>>>> Note: A child should include atleast a pin function selection property or
>>>> pin configuration property (one or more) or both.
>>>> + Note: Special property "samsung,off-state" indicates that this state can
>>>> + be used for device's "sleep" pins state. Its pin values/properties should
>>>> + match the configuration in power down mode.
>>> Why power down values cannot be used for sleep state? Why you need
>>> separate pin control state? If pins values should match power down
>>> configuration, then they could just be added to default state, couldn't
>>> they?
>> Separate sleep state is needed because of the pin control bindings and
>> design.
>>
>> A separate sleep state is required to let pin control client driver (in this
>> case
>> Exynos I2S driver) let to choose when it is okay to switch pads into sleep
>> state and I see no other way to achieve this.
> Maybe the pinctrl API should be extended for this? Doing this in DTS
> just for purpose of passing information between drivers (consumer and
> provider) looks odd.
Well, I don't know if it is odd or not, but that's how it is used now
and I see
no reason to reinvent wheel. Please check it yourself:
$ git grep \"sleep\" arch/arm/boot/dts | wc -l
101
> Anyway, you are proposing a new binding so please Cc devicetree mailing
> list and device tree maintainers.
I'm just using the generic pinctrl bindings, but it might make some sense to
add a note to Exynos i2s driver that a sleep pin control state is needed if
one wants to get power domain to be turned off.
>>> In the patch 2/9, existing configuration:
>>> 716 i2s0_bus: i2s0-bus {
>>> (...)
>>> 719 samsung,pin-function = <EXYNOS_PIN_FUNC_2>;
>>> 720 samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>> 721 samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>;
>>> 722 };
>>>
>>> additional configuration:
>>> + i2s0_bus_slp: i2s0-bus-slp {
>>> + samsung,pin-function = <EXYNOS_PIN_FUNC_INPUT>;
>>> + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>> + samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>;
>>> + samsung,pin-con-pdn = <EXYNOS_PIN_PDN_INPUT>;
>>> + samsung,pin-pud-pdn = <EXYNOS_PIN_PULL_NONE>;
>>> + samsung,off-state;
>>> + };
>> I agree that it would be possible to get rid of "samsung,off-state" property
>> and
>> just detect off state when func/pud pair matches power down func/pud.
>>
>>>> It indicates that pin control
>>>> + can notify runtime power management subsystem, that it is ready for runtime
>>>> + suspend if its all pins are configured for such state. This in turn might
>>>> + allow to turn respective power domain off to reduce power consumption.
>>> What do you mean by "notifying RPM subsystem"? Either this is
>>> description of hardware in certain mode (sleep state) or this is not
>>> device tree property.
>> Maybe I wrote the description with a view too limited to the kernel
>> operation
>> perspective, but if we remove the need to mark state as off, the above
>> description
>> will not be needed.
> But still it wouldn't be describing any hardware property, would it?
Well, it somehow describes the hardware, because the pin state when it
is allowed
to turn off the power domain is board specific. I should move that part
to the
Odroid dts, because there might be a board which require other pin
values in power
down mode.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox