* Re: [PATCH v3 2/6] mfd: dt: ranges, #address-cells and #size-cells as optional properties
From: Lee Jones @ 2017-01-03 17:49 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Rob Herring, Mark Rutland, Linus Walleij, Corey Minyard,
Cédric Le Goater, Joel Stanley, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20161206025321.1792-3-andrew@aj.id.au>
On Tue, 06 Dec 2016, Andrew Jeffery wrote:
> Whilst describing a device and not a bus, simple-mfd is modelled on
> simple-bus where child nodes are iterated and registered as platform
> devices. Some complex devices, e.g. the Aspeed LPC controller, can
> benefit from address space mapping such that child nodes can use the
> regs property to describe their resources within the multi-function
> device.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Applied with Acks, thanks.
> ---
> Documentation/devicetree/bindings/mfd/mfd.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> index f1fceeda12f1..bcb6abb9d413 100644
> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -25,6 +25,16 @@ Optional properties:
> be used. In the latter case the child devices will be determined by the
> operating system.
>
> +- ranges: Describes the address mapping relationship to the parent. Should set
> + the child's base address to 0, the physical address within parent's address
> + space, and the length of the address map.
> +
> +- #address-cells: Specifies the number of cells used to represent physical base
> + addresses. Must be present if ranges is used.
> +
> +- #size-cells: Specifies the number of cells used to represent the size of an
> + address. Must be present if ranges is used.
> +
> Example:
>
> foo@1000 {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3 3/6] mfd: dt: Add Aspeed Low Pin Count Controller bindings
From: Lee Jones @ 2017-01-03 17:49 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Rob Herring, Mark Rutland, Linus Walleij, Corey Minyard,
Cédric Le Goater, Joel Stanley,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161206025321.1792-4-andrew-zrmu5oMJ5Fs@public.gmane.org>
On Tue, 06 Dec 2016, Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Applied with Acks, thanks.
> ---
> .../devicetree/bindings/mfd/aspeed-lpc.txt | 111 +++++++++++++++++++++
> 1 file changed, 111 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> new file mode 100644
> index 000000000000..a97131aba446
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> @@ -0,0 +1,111 @@
> +======================================================================
> +Device tree bindings for the Aspeed Low Pin Count (LPC) Bus Controller
> +======================================================================
> +
> +The LPC bus is a means to bridge a host CPU to a number of low-bandwidth
> +peripheral devices, replacing the use of the ISA bus in the age of PCI[0]. The
> +primary use case of the Aspeed LPC controller is as a slave on the bus
> +(typically in a Baseboard Management Controller SoC), but under certain
> +conditions it can also take the role of bus master.
> +
> +The LPC controller is represented as a multi-function device to account for the
> +mix of functionality it provides. The principle split is between the register
> +layout at the start of the I/O space which is, to quote the Aspeed datasheet,
> +"basically compatible with the [LPC registers from the] popular BMC controller
> +H8S/2168[1]", and everything else, where everything else is an eclectic
> +collection of functions with a esoteric register layout. "Everything else",
> +here labeled the "host" portion of the controller, includes, but is not limited
> +to:
> +
> +* An IPMI Block Transfer[2] Controller
> +
> +* An LPC Host Controller: Manages LPC functions such as host vs slave mode, the
> + physical properties of some LPC pins, configuration of serial IRQs, and
> + APB-to-LPC bridging amonst other functions.
> +
> +* An LPC Host Interface Controller: Manages functions exposed to the host such
> + as LPC firmware hub cycles, configuration of the LPC-to-AHB mapping, UART
> + management and bus snoop configuration.
> +
> +* A set of SuperIO[3] scratch registers: Enables implementation of e.g. custom
> + hardware management protocols for handover between the host and baseboard
> + management controller.
> +
> +Additionally the state of the LPC controller influences the pinmux
> +configuration, therefore the host portion of the controller is exposed as a
> +syscon as a means to arbitrate access.
> +
> +[0] http://www.intel.com/design/chipsets/industry/25128901.pdf
> +[1] https://www.renesas.com/en-sg/doc/products/mpumcu/001/rej09b0078_h8s2168.pdf?key=7c88837454702128622bee53acbda8f4
> +[2] http://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/ipmi-second-gen-interface-spec-v2-rev1-1.pdf
> +[3] https://en.wikipedia.org/wiki/Super_I/O
> +
> +Required properties
> +===================
> +
> +- compatible: One of:
> + "aspeed,ast2400-lpc", "simple-mfd"
> + "aspeed,ast2500-lpc", "simple-mfd"
> +
> +- reg: contains the physical address and length values of the Aspeed
> + LPC memory region.
> +
> +- #address-cells: <1>
> +- #size-cells: <1>
> +- ranges: Maps 0 to the physical address and length of the LPC memory
> + region
> +
> +Required LPC Child nodes
> +========================
> +
> +BMC Node
> +--------
> +
> +- compatible: One of:
> + "aspeed,ast2400-lpc-bmc"
> + "aspeed,ast2500-lpc-bmc"
> +
> +- reg: contains the physical address and length values of the
> + H8S/2168-compatible LPC controller memory region
> +
> +Host Node
> +---------
> +
> +- compatible: One of:
> + "aspeed,ast2400-lpc-host", "simple-mfd", "syscon"
> + "aspeed,ast2500-lpc-host", "simple-mfd", "syscon"
> +
> +- reg: contains the address and length values of the host-related
> + register space for the Aspeed LPC controller
> +
> +- #address-cells: <1>
> +- #size-cells: <1>
> +- ranges: Maps 0 to the address and length of the host-related LPC memory
> + region
> +
> +Example:
> +
> +lpc: lpc@1e789000 {
> + compatible = "aspeed,ast2500-lpc", "simple-mfd";
> + reg = <0x1e789000 0x1000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x1e789000 0x1000>;
> +
> + lpc_bmc: lpc-bmc@0 {
> + compatible = "aspeed,ast2500-lpc-bmc";
> + reg = <0x0 0x80>;
> + };
> +
> + lpc_host: lpc-host@80 {
> + compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon";
> + reg = <0x80 0x1e0>;
> + reg-io-width = <4>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x0 0x80 0x1e0>;
> + };
> +};
> +
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 04/12] driver: clk: imx: Add clock driver for imx6sll
From: Rob Herring @ 2017-01-03 17:49 UTC (permalink / raw)
To: Bai Ping
Cc: shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
mark.rutland-5wv7dgnIgG8, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, fabio.estevam-3arQi8VN3Tc,
daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
tglx-hfZtesqFncYOwBW4kG4KsQ, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jacky.baip-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1482832070-22668-5-git-send-email-ping.bai-3arQi8VN3Tc@public.gmane.org>
On Tue, Dec 27, 2016 at 05:47:42PM +0800, Bai Ping wrote:
> Add clk driver support for imx6sll.
>
> Signed-off-by: Bai Ping <ping.bai-3arQi8VN3Tc@public.gmane.org>
> ---
> drivers/clk/imx/Makefile | 1 +
> drivers/clk/imx/clk-imx6sll.c | 369 ++++++++++++++++++++++++++++++
> include/dt-bindings/clock/imx6sll-clock.h | 204 +++++++++++++++++
This should be part of the binding doc patch.
> 3 files changed, 574 insertions(+)
> create mode 100644 drivers/clk/imx/clk-imx6sll.c
> create mode 100644 include/dt-bindings/clock/imx6sll-clock.h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 4/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LHC)
From: Lee Jones @ 2017-01-03 17:49 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Mark Rutland, devicetree, Corey Minyard, Linus Walleij,
linux-kernel, Rob Herring, Cédric Le Goater,
linux-arm-kernel, Joel Stanley
In-Reply-To: <20161206025321.1792-5-andrew@aj.id.au>
On Tue, 06 Dec 2016, Andrew Jeffery wrote:
> The LPC bus pinmux configuration on fifth generation Aspeed SoCs depends
> on bits in both the System Control Unit and the LPC Host Controller.
>
> The Aspeed LPC Host Controller is described as a child node of the
> LPC host-range syscon device for arbitration of access by the host
> controller and pinmux drivers.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Applied with Acks, thanks.
> ---
> .../devicetree/bindings/mfd/aspeed-lpc.txt | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> index a97131aba446..9de318ef72da 100644
> --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> @@ -109,3 +109,25 @@ lpc: lpc@1e789000 {
> };
> };
>
> +Host Node Children
> +==================
> +
> +LPC Host Controller
> +-------------------
> +
> +The Aspeed LPC Host Controller configures the Low Pin Count (LPC) bus behaviour
> +between the host and the baseboard management controller. The registers exist
> +in the "host" portion of the Aspeed LPC controller, which must be the parent of
> +the LPC host controller node.
> +
> +Required properties:
> +- compatible: "aspeed,ast2500-lhc";
> +- reg: contains offset/length value of the LHC memory
> + region.
> +
> +Example:
> +
> +lhc: lhc@20 {
> + compatible = "aspeed,ast2500-lhc";
> + reg = <0x20 0x24 0x48 0x8>;
> +};
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
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 v2 03/12] Document: dt: binding: imx: update clock doc for imx6sll
From: Rob Herring @ 2017-01-03 17:48 UTC (permalink / raw)
To: Bai Ping
Cc: mark.rutland, devicetree, p.zabel, mturquette, daniel.lezcano,
sboyd, linux-clk, linux-gpio, kernel, jacky.baip, fabio.estevam,
tglx, shawnguo, linus.walleij, linux-arm-kernel
In-Reply-To: <1482832070-22668-4-git-send-email-ping.bai@nxp.com>
On Tue, Dec 27, 2016 at 05:47:41PM +0800, Bai Ping wrote:
> Add clock binding doc uppdate for imx6sll.
s/uppdate/update/
>
> Signed-off-by: Bai Ping <ping.bai@nxp.com>
> ---
> .../devicetree/bindings/clock/imx6sll-clock.txt | 37 ++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/imx6sll-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/imx6sll-clock.txt b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> new file mode 100644
> index 0000000..3a46787
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> @@ -0,0 +1,37 @@
> +* Clock bindings for Freescale i.MX6 SLL
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6sll-ccm"
> +- reg: Address and length of the register set
> +- #clock-cells: Should be <1>
> +- clocks: list of clock specifiers, must contain an entry for each required
> + entry in clock-names
> +- clock-names: should include entries "ckil", "osc", "ipp_di0" and "ipp_di1"
> +
> +The clock consumer should specify the desired clock by having the clock
> +ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx6sll-clock.h
> +for the full list of i.MX6 SLL clock IDs.
> +
> +Examples:
> +
> +#include <dt-bindings/clock/imx6sll-clock.h>
> +
> +clks: ccm@020c4000 {
Drop leading 0s.
> + compatible = "fsl,imx6sll-ccm";
> + reg = <0x020c4000 0x4000>;
> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
> + #clock-cells = <1>;
> + clocks = <&ckil>, <&osc>, <&ipp_di0>, <&ipp_di1>;
> + clock-names = "ckil", "osc", "ipp_di0", "ipp_di1";
> +};
> +
> +uart1: serial@02020000 {
ditto.
> + compatible = "fsl,imx6sl-uart", "fsl,imx6q-uart", "fsl,imx21-uart";
> + reg = <0x02020000 0x4000>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6SLL_CLK_UART1_IPG>,
> + <&clks IMX6SLL_CLK_UART1_SERIAL>;
> + clock-names = "ipg", "per";
> + status = "disabled";
> +};
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 8/9] irqchip/ls-scfg-msi: add LS1043a v1.1 MSI support
From: Rob Herring @ 2017-01-03 17:46 UTC (permalink / raw)
To: Minghuan Lian
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Marc Zyngier, Jason Cooper,
Roy Zang, Mingkai Hu, Stuart Yoder, Yang-Leo Li, Scott Wood
In-Reply-To: <1482829985-24421-8-git-send-email-Minghuan.Lian-3arQi8VN3Tc@public.gmane.org>
On Tue, Dec 27, 2016 at 05:13:04PM +0800, Minghuan Lian wrote:
> A MSI controller of LS1043a v1.0 only includes one MSIR and
> is assigned one GIC interrupt. In order to support affinity,
> LS1043a v1.1 MSI is assigned 4 MSIRs and 4 GIC interrupts.
> But the MSIR has the different offset and only supports 8 MSIs.
> The bits between variable bit_start and bit_end in structure
> ls_scfg_msir are used to show 8 MSI interrupts. msir_irqs and
> msir_base are added to describe the difference of MSI between
> LS1043a v1.1 and other SoCs.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian-3arQi8VN3Tc@public.gmane.org>
> ---
> .../interrupt-controller/fsl,ls-scfg-msi.txt | 1 +
> drivers/irqchip/irq-ls-scfg-msi.c | 45 +++++++++++++++++++---
> 2 files changed, 40 insertions(+), 6 deletions(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 7/9] irqchip/ls-scfg-msi: add LS1046a MSI support
From: Rob Herring @ 2017-01-03 17:44 UTC (permalink / raw)
To: Minghuan Lian
Cc: devicetree, Jason Cooper, Roy Zang, Marc Zyngier, linux-kernel,
Stuart Yoder, Mingkai Hu, Scott Wood, Yang-Leo Li,
linux-arm-kernel
In-Reply-To: <1482829985-24421-7-git-send-email-Minghuan.Lian@nxp.com>
On Tue, Dec 27, 2016 at 05:13:03PM +0800, Minghuan Lian wrote:
> LS1046a includes 4 MSIRs, each MSIR is assigned a dedicate GIC
> SPI interrupt and provides 32 MSI interrupts. Compared to previous
> MSI, LS1046a's IBS(interrupt bit select) shift is changed to 2 and
> total MSI interrupt number is changed to 128.
>
> The patch adds structure 'ls_scfg_msir' to describe MSIR setting and
> 'ibs_shift' to store the different value between the SoCs.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> .../interrupt-controller/fsl,ls-scfg-msi.txt | 2 +-
> drivers/irqchip/irq-ls-scfg-msi.c | 161 ++++++++++++++++-----
> 2 files changed, 127 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> index 54597b0..dde4552 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> @@ -6,7 +6,7 @@ Required properties:
> Layerscape PCIe MSI controller block such as:
> "fsl,ls1021a-msi"
> "fsl,ls1043a-msi"
> - "fsl,ls1046a-msi"
> + "fsl,ls1046a-msi"
This hunk goes in the previous patch...
> - msi-controller: indicates that this is a PCIe MSI controller node
> - reg: physical base address of the controller and length of memory mapped.
> - interrupts: an interrupt to the parent interrupt controller.
^ permalink raw reply
* Re: [PATCH 0/4 v2] of/overlay: sysfs based ABI for dt overlays
From: Heinrich Schuchardt @ 2017-01-03 17:25 UTC (permalink / raw)
To: Pantelis Antoniou, Frank Rowand
Cc: Rob Herring, Mark Rutland, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ADF59727-1808-4C9E-B763-FF1F493DAACE-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On 01/03/2017 01:11 PM, Pantelis Antoniou wrote:
> Hi Frank, Heinrich,
>
>> On Dec 22, 2016, at 21:00 , Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> Hi Heinrich,
>>
>> On 12/20/16 11:04, Heinrich Schuchardt wrote:
>>> Currently the kernel only supplies an internal API for creating
>>> and destroying device tree overlays.
>>>
>>> For some boards vendor specific kernel modules exist for
>>> managing device tree overlays but they have not been
>>> upstreamed or upstreaming stalled.
>>> https://lkml.org/lkml/2015/6/12/624
>>> https://lkml.org/lkml/2013/1/7/366
>>>
>>> This patch series provides a sysfs based ABI for creation and
>>> destruction of dt overlays in /sys/firmware/devicetree/overlays.
>>>
>>> The following files are provided:
>>>
>>> load: This is a write only file.
>>> A string written to it is interpreted as the path to a
>>> flattened device tree overlay file. It is used to create
>>> and apply the contained overlays.
>>>
>>> loaded: This is a read only file.
>>> It provides the count of loaded overlays as a decimal
>>> number.
>>>
>>> unload: This is a write only file.
>>> If a positive number n is wrtten to this file the n
>>> most recent overlays are destroyed.
>>> If a negative number is written to this file all
>>> overlays are destroyed.
>>
>> This patch series follows a _somewhat_ similar approach to what
>> was first proposed two years ago, and does not address the
>> issues that were brought up at that time. See:
>>
>> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> Date: Wed, 3 Dec 2014 13:23:28 +0200
>> Subject: [PATCH] OF: DT-Overlay configfs interface (v3)
>>
>> But just responding directly to the two year old issues would not
>> be a productive approach, since there has been a lot of subsequent
>> discussion on how to load overlays (you point to two of the many
>> threads above). The latest discussions are based on the concept
>> of describing the overlay attachment points as connectors.
>>
>> Please join in pushing the connectors discussion along to make
>> sure that it meets your needs.
>>
>
> I think it would be best if we focus on getting the configfs based loader
> to work. It is pretty similar to what Heinrich is proposing.
>
>> -Frank
>>
>
> Regards
>
> — Pantelis
>
Reading Documentation/filesystems/configfs/configfs.txt it is not
evident to me that using configfs would be more appropriate than using
sysfs.
But I think before going into implementation details it is necessary to
define the scope of the development.
Up to now I have seen the following possible usages of device tree overlays:
1) Manage virtual devices
e.g. hot plugging CPUs and memory of a virtual machine
2) Activating optional hardware without discovery mechanism,
e.g an RTC on the I2C bus, or an I2C Grove sensor. This may include
changing the GPIO multiplexing.
3) Activating optional hardware with a discovery mechanism,
e.g. Beaglebone Capes and Raspberry hats with an EEPROM that can be read
over the I2C bus.
For scenario 3 elaborate kernel plugins have been written that try to
handle the complete complexity in kernel code, e.g. the Beaglebone cape
manager.
The discussion about connectors is also putting a lot of complexity into
possible kernel code, cf.
https://lkml.org/lkml/2016/6/30/734
In my view the kernel ABI for loading and removing overlays should
follow the KISS principle.
In scenario 3 a possible alternative design would be to put most of the
complexity into the userland and only supply the reading of the EEPROMs
via I2C and the loading mechanism for overlays as kernel modules.
In my patch series I hence only provided three interface functions:
Push an overlay on the stack of overlays.
Pop an overlay from the stack.
Count the overlays.
I prefer pop to delete by id because it will always succeed. But of
cause delete by id could be added to the ABI.
When applying overlays to change the same property again and again the
overlay stack can be considered a memory leak. So we might want to add a
merge/commit function. That function would empty the overlay stack
without undoing the changes.
Best regards
Heinrich Schuchardt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 6/8] IIO: add STM32 timer trigger driver
From: Jonathan Cameron @ 2017-01-03 17:24 UTC (permalink / raw)
To: Benjamin Gaignard, Jonathan Cameron
Cc: Mark Rutland, devicetree, Lars-Peter Clausen, Alexandre Torgue,
Linux PWM List, linux-iio, Linus Walleij, Arnaud Pouliquen,
Linux Kernel Mailing List, robh+dt, Thierry Reding,
Peter Meerwald-Stadler, Hartmut Knaack, Gerald Baeza,
Fabrice Gasnier, Lee Jones, Linaro Kernel Mailman List,
linux-arm-kernel, Benjamin Gaignard
In-Reply-To: <CA+M3ks4RuhUiBTu=GS9_9S8i2y53-Lc_r9rCZ1piPPuooZ4+Eg@mail.gmail.com>
On 3 January 2017 12:59:20 GMT+00:00, Benjamin Gaignard <benjamin.gaignard@linaro.org> wrote:
>2017-01-03 10:23 GMT+01:00 Benjamin Gaignard
><benjamin.gaignard@linaro.org>:
>> 2017-01-02 19:22 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>>> On 02/01/17 08:46, Benjamin Gaignard wrote:
>>>> 2016-12-30 22:12 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>>>>> On 09/12/16 14:15, Benjamin Gaignard wrote:
>>>>>> Timers IPs can be used to generate triggers for other IPs like
>>>>>> DAC, ADC or other timers.
>>>>>> Each trigger may result of timer internals signals like counter
>enable,
>>>>>> reset or edge, this configuration could be done through
>"master_mode"
>>>>>> device attribute.
>>>>>>
>>>>>> A timer device could be triggered by other timers, we use the
>trigger
>>>>>> name and is_stm32_iio_timer_trigger() function to distinguish
>them
>>>>>> and configure IP input switch.
>>>>>>
>>>>>> Timer may also decide on which event (edge, level) they could
>>>>>> be activated by a trigger, this configuration is done by writing
>in
>>>>>> "slave_mode" device attribute.
>>>>>>
>>>>>> Since triggers could also be used by DAC or ADC their names are
>defined
>>>>>> in include/ nux/iio/timer/stm32-timer-trigger.h so those IPs will
>be able
>>>>>> to configure themselves in valid_trigger function
>>>>>>
>>>>>> Trigger have a "sampling_frequency" attribute which allow to
>configure
>>>>>> timer sampling frequency without using PWM interface
>>>>>>
>>>>>> version 5:
>>>>>> - simplify tables of triggers
>>>>>> - only create an IIO device when needed
>>>>>>
>>>>>> version 4:
>>>>>> - get triggers configuration from "reg" in DT
>>>>>> - add tables of triggers
>>>>>> - sampling frequency is enable/disable when writing in trigger
>>>>>> sampling_frequency attribute
>>>>>> - no more use of interruptions
>>>>>>
>>>>>> version 3:
>>>>>> - change compatible to "st,stm32-timer-trigger"
>>>>>> - fix attributes access right
>>>>>> - use string instead of int for master_mode and slave_mode
>>>>>> - document device attributes in sysfs-bus-iio-timer-stm32
>>>>>>
>>>>>> version 2:
>>>>>> - keep only one compatible
>>>>>> - use st,input-triggers-names and st,output-triggers-names
>>>>>> to know which triggers are accepted and/or create by the device
>>>>> Firstly, sorry it has taken me so long to get back to this.
>>>>>
>>>>> I'm still not keen on this use of iio_device elements just to act
>as
>>>>> glue between triggers. I think we need to work out a more light
>weight
>>>>> way to do this. As you are only using them for validation and to
>provide
>>>>> somewhere to hang the control attibutes off, there is nothing
>stopping us
>>>>> moving that over to the iio_trigger instead which would avoid the
>messy
>>>>> duality going on here.
>>>>
>>>> I have add an iio_device because each hardware can generate
>multiple
>>>> triggers (up to 5: trgo, ch 1...4) and slave_mode attribute will
>impact all the
>>>> triggers of a device. For me it was making sense to centralize that
>in an
>>>> iio_device rather than having an attribute "shared" (from hardware
>>>> point of view)
>>>> on multiple triggers.
>>>> Since master_mode attribute is only used by trgo and not impact
>ch1...4
>>>> triggers I will move it to trigger instead of the iio_device.
>>>>
>>>> I also wanted to be able to connect triggers on a iio_device as I
>>>> could do for an
>>>> ADC with a command like 'echo "tim1_trgo" >
>iio_deviceX/trigger/current_trigger'
>>> This is interesting, but with a bit of refactoring I would think it
>would
>>> be possible to share some of that code thus allowing non IIO devices
>to
>>> bind to triggers. Ultimately I want to be able to bind a trigger to
>>> a trigger - I appreciate here the topology is more limited than that
>>> so some complexity comes in.
>>>
>>> My gut feeling is that representing that topology explicitly is hard
>>> to do in a remotely general way, but lets try it and see.
>>> We run into this sort of interdependency issue between different
>bits of
>>> the hardware all the time. Setting a value somewhere effects the
>configuration
>>> elsewhere - often the best plan is to just let that happen and leave
>it up to
>>> userspace to check for changes if it cares.
>>
>> okay
>>
>>>> If I change that to parent_trigger attribute it change this
>behavior
>>>> and I will have to
>>>> duplicated what is done in iio_trigger_write_current() to find and
>>>> validate triggers.
>>> I get the reasoning, but we still end up with something represented
>>> by an IIO device that isn't providing any channels at all. It's
>simply
>>> using some of the infrastructure. To my mind it is 'something else'
>>> and should be represented as such. I have no problem at all with
>>> you registering additional elements in /sysfs/bus/iio/ to represent
>>> these shared elements - we already have drivers that do that to
>>> provide some centralized infrastructure (e.g. the sysfs-trigger)
>>
>> My hardware block are timers maybe I can add a channel type
>"IIO_TIMER"
>> and declare a channel with info_mask_separate =
>BIT(IIO_CHAN_INFO_SAMP_FREQ)
>> so I will be able to write/read sampling frequency on IIO device.
>>
>
>If it isn't possible to implement IIO_TIMER I will simply drop device
>part of
>my driver until we find a solution because I would like to upstream at
>least what is
>need to ADC/DAC.
Agreed. Let's move forward with the easy stuff.
I might get a chance to look at trigger trees later
this week depending on how some tedious time in a pharma clean room and on planes goes.
I think we can mock most of this up with software triggers and the dummy driver.
Perhaps a sysfs trigger driving a timer trigger will get us moving in the right direction API wise.
Might let us figure out something generic without needing hardware.
A few other bits of hardware can do similar to yours but I don't think I have any in my pile of boards.
J
>
>>> I'm worried about the scope spread we get for an IIO device
>otherwise.
>>> They serve a well defined purpose at the moment, and that isn't what
>>> is happening here.
>>>
>>> So my gut feeling is we are better deliberately not representing the
>>> inter dependence and claiming all triggers we are creating are
>>> independent. That way we can have a nice generic infrastructure
>>> that will work in all cases (be it pushing the sanity checking to
>>> userspace).
>>>
>>> So each trigger has direct access to what controls it. Changing
>anything
>>> can effect other triggers in weird ways.
>>>
>>> I'm finding it hard to see anything else generalizing sufficiently
>>> as we'll always get cases where we can't represent the topology
>without
>>> diving into the complexity of something like the media controller
>>> framework.
>>>
>>> Jonathan
>>>>
>>>>> I might still be missing something though!
>>>>>
>>>>> You would only I think need 3 attributes
>>>>>
>>>>> parrent_trigger
>>>>> and something like your master_mode and slave_mode attributes.
>>>>>
>>>>> The parrent_trigger would need some validation etc, but if we keep
>it
>>>>> within this driver initially that won't be hard to do. Checking
>the device
>>>>> parent matches will do most of it.
>>>>>
>>>>> Jonathan
>>>>>>
>>>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>>> ---
>>>>>> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 55 +++
>>>>>> drivers/iio/Kconfig | 2 +-
>>>>>> drivers/iio/Makefile | 1 +
>>>>>> drivers/iio/timer/Kconfig | 13 +
>>>>>> drivers/iio/timer/Makefile | 1 +
>>>>>> drivers/iio/timer/stm32-timer-trigger.c | 466
>+++++++++++++++++++++
>>>>>> drivers/iio/trigger/Kconfig | 1 -
>>>>>> include/linux/iio/timer/stm32-timer-trigger.h | 62 +++
>>>>>> 8 files changed, 599 insertions(+), 2 deletions(-)
>>>>>> create mode 100644
>Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>>> create mode 100644 drivers/iio/timer/Kconfig
>>>>>> create mode 100644 drivers/iio/timer/Makefile
>>>>>> create mode 100644 drivers/iio/timer/stm32-timer-trigger.c
>>>>>> create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>>> new file mode 100644
>>>>>> index 0000000..26583dd
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>>> @@ -0,0 +1,55 @@
>>>>>> +What:
>/sys/bus/iio/devices/iio:deviceX/master_mode_available
>>>>>> +KernelVersion: 4.10
>>>>>> +Contact: benjamin.gaignard@st.com
>>>>>> +Description:
>>>>>> + Reading returns the list possible master modes
>which are:
>>>>>> + - "reset" : The UG bit from the TIMx_EGR
>register is used as trigger output (TRGO).
>>>>>> + - "enable" : The Counter Enable signal CNT_EN is
>used as trigger output.
>>>>>> + - "update" : The update event is selected as
>trigger output.
>>>>>> + For instance a master timer can
>then be used as a prescaler for a slave timer.
>>>>>> + - "compare_pulse" : The trigger output send a
>positive pulse when the CC1IF flag is to be set.
>>>>>> + - "OC1REF" : OC1REF signal is used as trigger
>output.
>>>>>> + - "OC2REF" : OC2REF signal is used as trigger
>output.
>>>>>> + - "OC3REF" : OC3REF signal is used as trigger
>output.
>>>>>> + - "OC4REF" : OC4REF signal is used as trigger
>output.
>>>>>> +
>>>>>> +What:
>/sys/bus/iio/devices/iio:deviceX/master_mode
>>>>>> +KernelVersion: 4.10
>>>>>> +Contact: benjamin.gaignard@st.com
>>>>>> +Description:
>>>>>> + Reading returns the current master modes.
>>>>>> + Writing set the master mode
>>>>>> +
>>>>>> +What:
>/sys/bus/iio/devices/iio:deviceX/slave_mode_available
>>>>>> +KernelVersion: 4.10
>>>>>> +Contact: benjamin.gaignard@st.com
>>>>>> +Description:
>>>>>> + Reading returns the list possible slave modes which
>are:
>>>>>> + - "disabled" : The prescaler is clocked directly
>by the internal clock.
>>>>>> + - "encoder_1" : Counter counts up/down on TI2FP1
>edge depending on TI1FP2 level.
>>>>>> + - "encoder_2" : Counter counts up/down on TI1FP2
>edge depending on TI2FP1 level.
>>>>>> + - "encoder_3" : Counter counts up/down on both
>TI1FP1 and TI2FP2 edges depending
>>>>>> + on the level of the other input.
>>>>>> + - "reset" : Rising edge of the selected trigger
>input reinitializes the counter
>>>>>> + and generates an update of the
>registers.
>>>>>> + - "gated" : The counter clock is enabled when
>the trigger input is high.
>>>>>> + The counter stops (but is not
>reset) as soon as the trigger becomes low.
>>>>>> + Both start and stop of the counter
>are controlled.
>>>>>> + - "trigger" : The counter starts at a rising edge
>of the trigger TRGI (but it is not
>>>>>> + reset). Only the start of the
>counter is controlled.
>>>>>> + - "external_clock": Rising edges of the selected
>trigger (TRGI) clock the counter.
>>>>>> +
>>>>>> +What: /sys/bus/iio/devices/iio:deviceX/slave_mode
>>>>>> +KernelVersion: 4.10
>>>>>> +Contact: benjamin.gaignard@st.com
>>>>>> +Description:
>>>>>> + Reading returns the current slave mode.
>>>>>> + Writing set the slave mode
>>>>>> +
>>>>>> +What:
>/sys/bus/iio/devices/triggerX/sampling_frequency
>>>>>> +KernelVersion: 4.10
>>>>>> +Contact: benjamin.gaignard@st.com
>>>>>> +Description:
>>>>>> + Reading returns the current sampling frequency.
>>>>>> + Writing an value different of 0 set and start
>sampling.
>>>>>> + Writing 0 stop sampling.
>>>>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>>>>>> index 6743b18..2de2a80 100644
>>>>>> --- a/drivers/iio/Kconfig
>>>>>> +++ b/drivers/iio/Kconfig
>>>>>> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig"
>>>>>> source "drivers/iio/pressure/Kconfig"
>>>>>> source "drivers/iio/proximity/Kconfig"
>>>>>> source "drivers/iio/temperature/Kconfig"
>>>>>> -
>>>>>> +source "drivers/iio/timer/Kconfig"
>>>>>> endif # IIO
>>>>>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>>>>>> index 87e4c43..b797c08 100644
>>>>>> --- a/drivers/iio/Makefile
>>>>>> +++ b/drivers/iio/Makefile
>>>>>> @@ -32,4 +32,5 @@ obj-y += potentiometer/
>>>>>> obj-y += pressure/
>>>>>> obj-y += proximity/
>>>>>> obj-y += temperature/
>>>>>> +obj-y += timer/
>>>>>> obj-y += trigger/
>>>>>> diff --git a/drivers/iio/timer/Kconfig
>b/drivers/iio/timer/Kconfig
>>>>>> new file mode 100644
>>>>>> index 0000000..e3c21f2
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/iio/timer/Kconfig
>>>>>> @@ -0,0 +1,13 @@
>>>>>> +#
>>>>>> +# Timers drivers
>>>>>> +
>>>>>> +menu "Timers"
>>>>>> +
>>>>>> +config IIO_STM32_TIMER_TRIGGER
>>>>>> + tristate "STM32 Timer Trigger"
>>>>>> + depends on (ARCH_STM32 && OF && MFD_STM32_TIMERS) ||
>COMPILE_TEST
>>>>>> + select IIO_TRIGGERED_EVENT
>>>>>> + help
>>>>>> + Select this option to enable STM32 Timer Trigger
>>>>>> +
>>>>>> +endmenu
>>>>>> diff --git a/drivers/iio/timer/Makefile
>b/drivers/iio/timer/Makefile
>>>>>> new file mode 100644
>>>>>> index 0000000..4ad95ec9
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/iio/timer/Makefile
>>>>>> @@ -0,0 +1 @@
>>>>>> +obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o
>>>>>> diff --git a/drivers/iio/timer/stm32-timer-trigger.c
>b/drivers/iio/timer/stm32-timer-trigger.c
>>>>>> new file mode 100644
>>>>>> index 0000000..8d16e8f
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/iio/timer/stm32-timer-trigger.c
>>>>>> @@ -0,0 +1,466 @@
>>>>>> +/*
>>>>>> + * Copyright (C) STMicroelectronics 2016
>>>>>> + *
>>>>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>>> + *
>>>>>> + * License terms: GNU General Public License (GPL), version 2
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/iio/iio.h>
>>>>>> +#include <linux/iio/sysfs.h>
>>>>>> +#include <linux/iio/timer/stm32-timer-trigger.h>
>>>>>> +#include <linux/iio/trigger.h>
>>>>>> +#include <linux/iio/triggered_event.h>
>>>>>> +#include <linux/interrupt.h>
>>>>>> +#include <linux/mfd/stm32-timers.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +
>>>>>> +#define MAX_TRIGGERS 6
>>>>>> +#define MAX_VALIDS 5
>>>>>> +
>>>>>> +/* List the triggers created by each timer */
>>>>>> +static const void *triggers_table[][MAX_TRIGGERS] = {
>>>>>> + { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
>>>>>> + { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
>>>>>> + { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
>>>>>> + { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
>>>>>> + { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
>>>>>> + { TIM6_TRGO,},
>>>>>> + { TIM7_TRGO,},
>>>>>> + { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
>>>>>> + { TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
>>>>>> + { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>>>>>> +};
>>>>>> +
>>>>>> +/* List the triggers accepted by each timer */
>>>>>> +static const void *valids_table[][MAX_VALIDS] = {
>>>>>> + { TIM5_TRGO, TIM2_TRGO, TIM4_TRGO, TIM3_TRGO,},
>>>>>> + { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>>>>> + { TIM1_TRGO, TIM8_TRGO, TIM5_TRGO, TIM4_TRGO,},
>>>>>> + { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>>>>>> + { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>>>>>> + { }, /* timer 6 */
>>>>>> + { }, /* timer 7 */
>>>>>> + { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>>>>>> + { TIM2_TRGO, TIM3_TRGO,},
>>>>>> + { TIM4_TRGO, TIM5_TRGO,},
>>>>>> +};
>>>>>> +
>>>>>> +struct stm32_timer_trigger {
>>>>>> + struct device *dev;
>>>>>> + struct regmap *regmap;
>>>>>> + struct clk *clk;
>>>>>> + u32 max_arr;
>>>>>> + const void *triggers;
>>>>>> + const void *valids;
>>>>>> +};
>>>>>> +
>>>>>> +static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>>>>> + unsigned int frequency)
>>>>>> +{
>>>>>> + unsigned long long prd, div;
>>>>>> + int prescaler = 0;
>>>>>> + u32 ccer, cr1;
>>>>>> +
>>>>>> + /* Period and prescaler values depends of clock rate */
>>>>>> + div = (unsigned long long)clk_get_rate(priv->clk);
>>>>>> +
>>>>>> + do_div(div, frequency);
>>>>>> +
>>>>>> + prd = div;
>>>>>> +
>>>>>> + /*
>>>>>> + * Increase prescaler value until we get a result that fit
>>>>>> + * with auto reload register maximum value.
>>>>>> + */
>>>>>> + while (div > priv->max_arr) {
>>>>>> + prescaler++;
>>>>>> + div = prd;
>>>>>> + do_div(div, (prescaler + 1));
>>>>>> + }
>>>>>> + prd = div;
>>>>>> +
>>>>>> + if (prescaler > MAX_TIM_PSC) {
>>>>>> + dev_err(priv->dev, "prescaler exceeds the maximum
>value\n");
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> + /* Check if nobody else use the timer */
>>>>>> + regmap_read(priv->regmap, TIM_CCER, &ccer);
>>>>>> + if (ccer & TIM_CCER_CCXE)
>>>>>> + return -EBUSY;
>>>>>> +
>>>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>>>> + if (!(cr1 & TIM_CR1_CEN))
>>>>>> + clk_enable(priv->clk);
>>>>>> +
>>>>>> + regmap_write(priv->regmap, TIM_PSC, prescaler);
>>>>>> + regmap_write(priv->regmap, TIM_ARR, prd - 1);
>>>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE,
>TIM_CR1_ARPE);
>>>>>> +
>>>>>> + /* Force master mode to update mode */
>>>>>> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS,
>0x20);
>>>>>> +
>>>>>> + /* Make sure that registers are updated */
>>>>>> + regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG,
>TIM_EGR_UG);
>>>>>> +
>>>>>> + /* Enable controller */
>>>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
>TIM_CR1_CEN);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void stm32_timer_stop(struct stm32_timer_trigger *priv)
>>>>>> +{
>>>>>> + u32 ccer, cr1;
>>>>>> +
>>>>>> + regmap_read(priv->regmap, TIM_CCER, &ccer);
>>>>>> + if (ccer & TIM_CCER_CCXE)
>>>>>> + return;
>>>>>> +
>>>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>>>> + if (cr1 & TIM_CR1_CEN)
>>>>>> + clk_disable(priv->clk);
>>>>>> +
>>>>>> + /* Stop timer */
>>>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
>>>>>> + regmap_write(priv->regmap, TIM_PSC, 0);
>>>>>> + regmap_write(priv->regmap, TIM_ARR, 0);
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t stm32_tt_store_frequency(struct device *dev,
>>>>>> + struct device_attribute
>*attr,
>>>>>> + const char *buf, size_t
>len)
>>>>>> +{
>>>>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>>>>> + struct stm32_timer_trigger *priv =
>iio_trigger_get_drvdata(trig);
>>>>>> + unsigned int freq;
>>>>>> + int ret;
>>>>>> +
>>>>>> + ret = kstrtouint(buf, 10, &freq);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + if (freq == 0) {
>>>>>> + stm32_timer_stop(priv);
>>>>>> + } else {
>>>>>> + ret = stm32_timer_start(priv, freq);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + }
>>>>>> +
>>>>>> + return len;
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t stm32_tt_read_frequency(struct device *dev,
>>>>>> + struct device_attribute
>*attr, char *buf)
>>>>>> +{
>>>>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>>>>> + struct stm32_timer_trigger *priv =
>iio_trigger_get_drvdata(trig);
>>>>>> + u32 psc, arr, cr1;
>>>>>> + unsigned long long freq = 0;
>>>>>> +
>>>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>>>> + regmap_read(priv->regmap, TIM_PSC, &psc);
>>>>>> + regmap_read(priv->regmap, TIM_ARR, &arr);
>>>>>> +
>>>>>> + if (psc && arr && (cr1 & TIM_CR1_CEN)) {
>>>>>> + freq = (unsigned long long)clk_get_rate(priv->clk);
>>>>>> + do_div(freq, psc);
>>>>>> + do_div(freq, arr);
>>>>>> + }
>>>>>> +
>>>>>> + return sprintf(buf, "%d\n", (unsigned int)freq);
>>>>>> +}
>>>>>> +
>>>>>> +static IIO_DEV_ATTR_SAMP_FREQ(0660,
>>>>>> + stm32_tt_read_frequency,
>>>>>> + stm32_tt_store_frequency);
>>>>>> +
>>>>>> +static struct attribute *stm32_trigger_attrs[] = {
>>>>>> + &iio_dev_attr_sampling_frequency.dev_attr.attr,
>>>>>> + NULL,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct attribute_group stm32_trigger_attr_group = {
>>>>>> + .attrs = stm32_trigger_attrs,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct attribute_group *stm32_trigger_attr_groups[]
>= {
>>>>>> + &stm32_trigger_attr_group,
>>>>>> + NULL,
>>>>>> +};
>>>>>> +
>>>>>> +static char *master_mode_table[] = {
>>>>>> + "reset",
>>>>>> + "enable",
>>>>>> + "update",
>>>>>> + "compare_pulse",
>>>>>> + "OC1REF",
>>>>>> + "OC2REF",
>>>>>> + "OC3REF",
>>>>>> + "OC4REF"
>>>>>> +};
>>>>>> +
>>>>>> +static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>>>>> + struct device_attribute
>*attr,
>>>>>> + char *buf)
>>>>>> +{
>>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>>> + u32 cr2;
>>>>>> +
>>>>>> + regmap_read(priv->regmap, TIM_CR2, &cr2);
>>>>>> + cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
>>>>>> +
>>>>>> + return snprintf(buf, PAGE_SIZE, "%s\n",
>master_mode_table[cr2]);
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>>>>> + struct device_attribute
>*attr,
>>>>>> + const char *buf, size_t
>len)
>>>>>> +{
>>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>>>>> + if (!strncmp(master_mode_table[i], buf,
>>>>>> + strlen(master_mode_table[i]))) {
>>>>>> + regmap_update_bits(priv->regmap, TIM_CR2,
>>>>>> + TIM_CR2_MMS, i <<
>TIM_CR2_MMS_SHIFT);
>>>>>> + return len;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + return -EINVAL;
>>>>>> +}
>>>>>> +
>>>>>> +static IIO_CONST_ATTR(master_mode_available,
>>>>>> + "reset enable update compare_pulse OC1REF OC2REF OC3REF
>OC4REF");
>>>>>> +
>>>>>> +static IIO_DEVICE_ATTR(master_mode, 0660,
>>>>>> + stm32_tt_show_master_mode,
>>>>>> + stm32_tt_store_master_mode,
>>>>>> + 0);
>>>>>> +
>>>>>> +static char *slave_mode_table[] = {
>>>>>> + "disabled",
>>>>>> + "encoder_1",
>>>>>> + "encoder_2",
>>>>>> + "encoder_3",
>>>>>> + "reset",
>>>>>> + "gated",
>>>>>> + "trigger",
>>>>>> + "external_clock",
>>>>>> +};
>>>>>> +
>>>>>> +static ssize_t stm32_tt_show_slave_mode(struct device *dev,
>>>>>> + struct device_attribute
>*attr,
>>>>>> + char *buf)
>>>>>> +{
>>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>>> + u32 smcr;
>>>>>> +
>>>>>> + regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>>>> + smcr &= TIM_SMCR_SMS;
>>>>>> +
>>>>>> + return snprintf(buf, PAGE_SIZE, "%s\n",
>slave_mode_table[smcr]);
>>>>>> +}
>>>>>> +
>>>>>> +static ssize_t stm32_tt_store_slave_mode(struct device *dev,
>>>>>> + struct device_attribute
>*attr,
>>>>>> + const char *buf, size_t
>len)
>>>>>> +{
>>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; i < ARRAY_SIZE(slave_mode_table); i++) {
>>>>>> + if (!strncmp(slave_mode_table[i], buf,
>>>>>> + strlen(slave_mode_table[i]))) {
>>>>>> + regmap_update_bits(priv->regmap,
>>>>>> + TIM_SMCR, TIM_SMCR_SMS,
>i);
>>>>>> + return len;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + return -EINVAL;
>>>>>> +}
>>>>>> +
>>>>>> +static IIO_CONST_ATTR(slave_mode_available,
>>>>>> +"disabled encoder_1 encoder_2 encoder_3 reset gated trigger
>external_clock");
>>>>>> +
>>>>>> +static IIO_DEVICE_ATTR(slave_mode, 0660,
>>>>>> + stm32_tt_show_slave_mode,
>>>>>> + stm32_tt_store_slave_mode,
>>>>>> + 0);
>>>>>> +
>>>>>> +static struct attribute *stm32_timer_attrs[] = {
>>>>>> + &iio_dev_attr_master_mode.dev_attr.attr,
>>>>>> + &iio_const_attr_master_mode_available.dev_attr.attr,
>>>>>> + &iio_dev_attr_slave_mode.dev_attr.attr,
>>>>>> + &iio_const_attr_slave_mode_available.dev_attr.attr,
>>>>>> + NULL,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct attribute_group stm32_timer_attr_group = {
>>>>>> + .attrs = stm32_timer_attrs,
>>>>>> +};
>>>>>> +
>>>>>> +static const struct iio_trigger_ops timer_trigger_ops = {
>>>>>> + .owner = THIS_MODULE,
>>>>>> +};
>>>>>> +
>>>>>> +static int stm32_setup_iio_triggers(struct stm32_timer_trigger
>*priv)
>>>>>> +{
>>>>>> + int ret;
>>>>>> + const char * const *cur = priv->triggers;
>>>>>> +
>>>>>> + while (cur && *cur) {
>>>>>> + struct iio_trigger *trig;
>>>>>> +
>>>>>> + trig = devm_iio_trigger_alloc(priv->dev, "%s",
>*cur);
>>>>>> + if (!trig)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + trig->dev.parent = priv->dev->parent;
>>>>>> + trig->ops = &timer_trigger_ops;
>>>>>> + trig->dev.groups = stm32_trigger_attr_groups;
>>>>>> + iio_trigger_set_drvdata(trig, priv);
>>>>>> +
>>>>>> + ret = devm_iio_trigger_register(priv->dev, trig);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> + cur++;
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +/**
>>>>>> + * is_stm32_timer_trigger
>>>>>> + * @trig: trigger to be checked
>>>>>> + *
>>>>>> + * return true if the trigger is a valid stm32 iio timer trigger
>>>>>> + * either return false
>>>>>> + */
>>>>>> +bool is_stm32_timer_trigger(struct iio_trigger *trig)
>>>>>> +{
>>>>>> + return (trig->ops == &timer_trigger_ops);
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(is_stm32_timer_trigger);
>>>>>> +
>>>>>> +static int stm32_validate_trigger(struct iio_dev *indio_dev,
>>>>>> + struct iio_trigger *trig)
>>>>>> +{
>>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>>> + const char * const *cur = priv->valids;
>>>>>> + unsigned int i = 0;
>>>>>> +
>>>>>> + if (!is_stm32_timer_trigger(trig))
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + while (cur && *cur) {
>>>>>> + if (!strncmp(trig->name, *cur, strlen(trig->name)))
>{
>>>>>> + regmap_update_bits(priv->regmap,
>>>>>> + TIM_SMCR, TIM_SMCR_TS,
>>>>>> + i << TIM_SMCR_TS_SHIFT);
>>>>>> + return 0;
>>>>>> + }
>>>>>> + cur++;
>>>>>> + i++;
>>>>>> + }
>>>>>> +
>>>>>> + return -EINVAL;
>>>>>> +}
>>>>>> +
>>>>>> +static const struct iio_info stm32_trigger_info = {
>>>>>> + .driver_module = THIS_MODULE,
>>>>>> + .validate_trigger = stm32_validate_trigger,
>>>>>> + .attrs = &stm32_timer_attr_group,
>>>>>> +};
>>>>>> +
>>>>>> +static struct stm32_timer_trigger *stm32_setup_iio_device(struct
>device *dev)
>>>>>> +{
>>>>>> + struct iio_dev *indio_dev;
>>>>>> + int ret;
>>>>>> +
>>>>>> + indio_dev = devm_iio_device_alloc(dev,
>>>>>> + sizeof(struct
>stm32_timer_trigger));
>>>>>> + if (!indio_dev)
>>>>>> + return NULL;
>>>>>> +
>>>>>> + indio_dev->name = dev_name(dev);
>>>>>> + indio_dev->dev.parent = dev;
>>>>>> + indio_dev->info = &stm32_trigger_info;
>>>>>> + indio_dev->modes = INDIO_EVENT_TRIGGERED;
>>>>>> + indio_dev->num_channels = 0;
>>>>>> + indio_dev->dev.of_node = dev->of_node;
>>>>>> +
>>>>>> + ret = devm_iio_device_register(dev, indio_dev);
>>>>>> + if (ret)
>>>>>> + return NULL;
>>>>>> +
>>>>>> + return iio_priv(indio_dev);
>>>>>> +}
>>>>>> +
>>>>>> +static int stm32_timer_trigger_probe(struct platform_device
>*pdev)
>>>>>> +{
>>>>>> + struct device *dev = &pdev->dev;
>>>>>> + struct stm32_timer_trigger *priv;
>>>>>> + struct stm32_timers *ddata =
>dev_get_drvdata(pdev->dev.parent);
>>>>>> + unsigned int index;
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (of_property_read_u32(dev->of_node, "reg", &index))
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + if (index >= ARRAY_SIZE(triggers_table))
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /* Create an IIO device only if we have triggers to be
>validated */
>>>>>> + if (*valids_table[index])
>>>>>> + priv = stm32_setup_iio_device(dev);
>>>>>
>>>>> I still don't like this. Really feels like we shouldn't be
>creating an
>>>>> iio device with all the bagage that carries just to allow us to do
>the
>>>>> trigger trees. We ought to have a much more light weight solution
>for this
>>>>> functionality - we aren't typically even using the interrupt tree
>stuff
>>>>> that the triggers for devices are all really about.
>>>>>
>>>>> A simpler approach of allowing each trigger the option of a parent
>seems like
>>>>> it would be cleaner. Could be done entirely within this driver in
>the first
>>>>> instance. Basically it would just look like your master and slave
>attributes
>>>>> but have those under triggerX not iio:deviceX.
>>>>>
>>>>> We can work out how to make it more generic later - including
>perhaps the
>>>>> option to trigger from triggers outside this driver, using some
>parallel
>>>>> infrastructure to the device triggering.
>>>>>
>>>>>
>>>>>> + else
>>>>>> + priv = devm_kzalloc(dev, sizeof(*priv),
>GFP_KERNEL);
>>>>>> +
>>>>>> + if (!priv)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + priv->dev = dev;
>>>>>> + priv->regmap = ddata->regmap;
>>>>>> + priv->clk = ddata->clk;
>>>>>> + priv->max_arr = ddata->max_arr;
>>>>>> + priv->triggers = triggers_table[index];
>>>>>> + priv->valids = valids_table[index];
>>>>>> +
>>>>>> + ret = stm32_setup_iio_triggers(priv);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>> +
>>>>>> + platform_set_drvdata(pdev, priv);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static const struct of_device_id stm32_trig_of_match[] = {
>>>>>> + { .compatible = "st,stm32-timer-trigger", },
>>>>>> + { /* end node */ },
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
>>>>>> +
>>>>>> +static struct platform_driver stm32_timer_trigger_driver = {
>>>>>> + .probe = stm32_timer_trigger_probe,
>>>>>> + .driver = {
>>>>>> + .name = "stm32-timer-trigger",
>>>>>> + .of_match_table = stm32_trig_of_match,
>>>>>> + },
>>>>>> +};
>>>>>> +module_platform_driver(stm32_timer_trigger_driver);
>>>>>> +
>>>>>> +MODULE_ALIAS("platform: stm32-timer-trigger");
>>>>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer Trigger
>driver");
>>>>>> +MODULE_LICENSE("GPL v2");
>>>>>> diff --git a/drivers/iio/trigger/Kconfig
>b/drivers/iio/trigger/Kconfig
>>>>>> index 809b2e7..f2af4fe 100644
>>>>>> --- a/drivers/iio/trigger/Kconfig
>>>>>> +++ b/drivers/iio/trigger/Kconfig
>>>>>> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER
>>>>>>
>>>>>> To compile this driver as a module, choose M here: the
>>>>>> module will be called iio-trig-sysfs.
>>>>>> -
>>>>> Clean this up.
>>>>
>>>> ok
>>>>
>>>>>> endmenu
>>>>>> diff --git a/include/linux/iio/timer/stm32-timer-trigger.h
>b/include/linux/iio/timer/stm32-timer-trigger.h
>>>>>> new file mode 100644
>>>>>> index 0000000..55535ae
>>>>>> --- /dev/null
>>>>>> +++ b/include/linux/iio/timer/stm32-timer-trigger.h
>>>>>> @@ -0,0 +1,62 @@
>>>>>> +/*
>>>>>> + * Copyright (C) STMicroelectronics 2016
>>>>>> + *
>>>>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>>> + *
>>>>>> + * License terms: GNU General Public License (GPL), version 2
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef _STM32_TIMER_TRIGGER_H_
>>>>>> +#define _STM32_TIMER_TRIGGER_H_
>>>>>> +
>>>>>> +#define TIM1_TRGO "tim1_trgo"
>>>>>> +#define TIM1_CH1 "tim1_ch1"
>>>>>> +#define TIM1_CH2 "tim1_ch2"
>>>>>> +#define TIM1_CH3 "tim1_ch3"
>>>>>> +#define TIM1_CH4 "tim1_ch4"
>>>>>> +
>>>>>> +#define TIM2_TRGO "tim2_trgo"
>>>>>> +#define TIM2_CH1 "tim2_ch1"
>>>>>> +#define TIM2_CH2 "tim2_ch2"
>>>>>> +#define TIM2_CH3 "tim2_ch3"
>>>>>> +#define TIM2_CH4 "tim2_ch4"
>>>>>> +
>>>>>> +#define TIM3_TRGO "tim3_trgo"
>>>>>> +#define TIM3_CH1 "tim3_ch1"
>>>>>> +#define TIM3_CH2 "tim3_ch2"
>>>>>> +#define TIM3_CH3 "tim3_ch3"
>>>>>> +#define TIM3_CH4 "tim3_ch4"
>>>>>> +
>>>>>> +#define TIM4_TRGO "tim4_trgo"
>>>>>> +#define TIM4_CH1 "tim4_ch1"
>>>>>> +#define TIM4_CH2 "tim4_ch2"
>>>>>> +#define TIM4_CH3 "tim4_ch3"
>>>>>> +#define TIM4_CH4 "tim4_ch4"
>>>>>> +
>>>>>> +#define TIM5_TRGO "tim5_trgo"
>>>>>> +#define TIM5_CH1 "tim5_ch1"
>>>>>> +#define TIM5_CH2 "tim5_ch2"
>>>>>> +#define TIM5_CH3 "tim5_ch3"
>>>>>> +#define TIM5_CH4 "tim5_ch4"
>>>>>> +
>>>>>> +#define TIM6_TRGO "tim6_trgo"
>>>>>> +
>>>>>> +#define TIM7_TRGO "tim7_trgo"
>>>>>> +
>>>>>> +#define TIM8_TRGO "tim8_trgo"
>>>>>> +#define TIM8_CH1 "tim8_ch1"
>>>>>> +#define TIM8_CH2 "tim8_ch2"
>>>>>> +#define TIM8_CH3 "tim8_ch3"
>>>>>> +#define TIM8_CH4 "tim8_ch4"
>>>>>> +
>>>>>> +#define TIM9_TRGO "tim9_trgo"
>>>>>> +#define TIM9_CH1 "tim9_ch1"
>>>>>> +#define TIM9_CH2 "tim9_ch2"
>>>>>> +
>>>>>> +#define TIM12_TRGO "tim12_trgo"
>>>>>> +#define TIM12_CH1 "tim12_ch1"
>>>>>> +#define TIM12_CH2 "tim12_ch2"
>>>>>> +
>>>>>> +bool is_stm32_timer_trigger(struct iio_trigger *trig);
>>>>>> +
>>>>>> +#endif
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>> --
>> Benjamin Gaignard
>>
>> Graphic Study Group
>>
>> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: Facebook | Twitter | Blog
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
_______________________________________________
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 v6 6/8] IIO: add STM32 timer trigger driver
From: Jonathan Cameron @ 2017-01-03 17:16 UTC (permalink / raw)
To: Benjamin Gaignard, Jonathan Cameron
Cc: Mark Rutland, devicetree, Lars-Peter Clausen, Alexandre Torgue,
Linux PWM List, linux-iio, Linus Walleij, Arnaud Pouliquen,
Linux Kernel Mailing List, robh+dt, Thierry Reding,
Peter Meerwald-Stadler, Hartmut Knaack, Gerald Baeza,
Fabrice Gasnier, Lee Jones, Linaro Kernel Mailman List,
linux-arm-kernel, Benjamin Gaignard
In-Reply-To: <CA+M3ks6jshX-rEsi7Qfo-65awzWcHsEMVBjm-baYy979V9187Q@mail.gmail.com>
On 3 January 2017 09:23:34 GMT+00:00, Benjamin Gaignard <benjamin.gaignard@linaro.org> wrote:
>2017-01-02 19:22 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>> On 02/01/17 08:46, Benjamin Gaignard wrote:
>>> 2016-12-30 22:12 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>>>> On 09/12/16 14:15, Benjamin Gaignard wrote:
>>>>> Timers IPs can be used to generate triggers for other IPs like
>>>>> DAC, ADC or other timers.
>>>>> Each trigger may result of timer internals signals like counter
>enable,
>>>>> reset or edge, this configuration could be done through
>"master_mode"
>>>>> device attribute.
>>>>>
>>>>> A timer device could be triggered by other timers, we use the
>trigger
>>>>> name and is_stm32_iio_timer_trigger() function to distinguish them
>>>>> and configure IP input switch.
>>>>>
>>>>> Timer may also decide on which event (edge, level) they could
>>>>> be activated by a trigger, this configuration is done by writing
>in
>>>>> "slave_mode" device attribute.
>>>>>
>>>>> Since triggers could also be used by DAC or ADC their names are
>defined
>>>>> in include/ nux/iio/timer/stm32-timer-trigger.h so those IPs will
>be able
>>>>> to configure themselves in valid_trigger function
>>>>>
>>>>> Trigger have a "sampling_frequency" attribute which allow to
>configure
>>>>> timer sampling frequency without using PWM interface
>>>>>
>>>>> version 5:
>>>>> - simplify tables of triggers
>>>>> - only create an IIO device when needed
>>>>>
>>>>> version 4:
>>>>> - get triggers configuration from "reg" in DT
>>>>> - add tables of triggers
>>>>> - sampling frequency is enable/disable when writing in trigger
>>>>> sampling_frequency attribute
>>>>> - no more use of interruptions
>>>>>
>>>>> version 3:
>>>>> - change compatible to "st,stm32-timer-trigger"
>>>>> - fix attributes access right
>>>>> - use string instead of int for master_mode and slave_mode
>>>>> - document device attributes in sysfs-bus-iio-timer-stm32
>>>>>
>>>>> version 2:
>>>>> - keep only one compatible
>>>>> - use st,input-triggers-names and st,output-triggers-names
>>>>> to know which triggers are accepted and/or create by the device
>>>> Firstly, sorry it has taken me so long to get back to this.
>>>>
>>>> I'm still not keen on this use of iio_device elements just to act
>as
>>>> glue between triggers. I think we need to work out a more light
>weight
>>>> way to do this. As you are only using them for validation and to
>provide
>>>> somewhere to hang the control attibutes off, there is nothing
>stopping us
>>>> moving that over to the iio_trigger instead which would avoid the
>messy
>>>> duality going on here.
>>>
>>> I have add an iio_device because each hardware can generate multiple
>>> triggers (up to 5: trgo, ch 1...4) and slave_mode attribute will
>impact all the
>>> triggers of a device. For me it was making sense to centralize that
>in an
>>> iio_device rather than having an attribute "shared" (from hardware
>>> point of view)
>>> on multiple triggers.
>>> Since master_mode attribute is only used by trgo and not impact
>ch1...4
>>> triggers I will move it to trigger instead of the iio_device.
>>>
>>> I also wanted to be able to connect triggers on a iio_device as I
>>> could do for an
>>> ADC with a command like 'echo "tim1_trgo" >
>iio_deviceX/trigger/current_trigger'
>> This is interesting, but with a bit of refactoring I would think it
>would
>> be possible to share some of that code thus allowing non IIO devices
>to
>> bind to triggers. Ultimately I want to be able to bind a trigger to
>> a trigger - I appreciate here the topology is more limited than that
>> so some complexity comes in.
>>
>> My gut feeling is that representing that topology explicitly is hard
>> to do in a remotely general way, but lets try it and see.
>> We run into this sort of interdependency issue between different bits
>of
>> the hardware all the time. Setting a value somewhere effects the
>configuration
>> elsewhere - often the best plan is to just let that happen and leave
>it up to
>> userspace to check for changes if it cares.
>
>okay
>
>>> If I change that to parent_trigger attribute it change this behavior
>>> and I will have to
>>> duplicated what is done in iio_trigger_write_current() to find and
>>> validate triggers.
>> I get the reasoning, but we still end up with something represented
>> by an IIO device that isn't providing any channels at all. It's
>simply
>> using some of the infrastructure. To my mind it is 'something else'
>> and should be represented as such. I have no problem at all with
>> you registering additional elements in /sysfs/bus/iio/ to represent
>> these shared elements - we already have drivers that do that to
>> provide some centralized infrastructure (e.g. the sysfs-trigger)
>
>My hardware block are timers maybe I can add a channel type "IIO_TIMER"
>and declare a channel with info_mask_separate =
>BIT(IIO_CHAN_INFO_SAMP_FREQ)
>so I will be able to write/read sampling frequency on IIO device.
Hmm stretching a point. There aren't really input or output channels.
Still not convinced there should be any IIO devices near this at all.
>
>> I'm worried about the scope spread we get for an IIO device
>otherwise.
>> They serve a well defined purpose at the moment, and that isn't what
>> is happening here.
>>
>> So my gut feeling is we are better deliberately not representing the
>> inter dependence and claiming all triggers we are creating are
>> independent. That way we can have a nice generic infrastructure
>> that will work in all cases (be it pushing the sanity checking to
>> userspace).
>>
>> So each trigger has direct access to what controls it. Changing
>anything
>> can effect other triggers in weird ways.
>>
>> I'm finding it hard to see anything else generalizing sufficiently
>> as we'll always get cases where we can't represent the topology
>without
>> diving into the complexity of something like the media controller
>> framework.
>>
>> Jonathan
>>>
>>>> I might still be missing something though!
>>>>
>>>> You would only I think need 3 attributes
>>>>
>>>> parrent_trigger
>>>> and something like your master_mode and slave_mode attributes.
>>>>
>>>> The parrent_trigger would need some validation etc, but if we keep
>it
>>>> within this driver initially that won't be hard to do. Checking the
>device
>>>> parent matches will do most of it.
>>>>
>>>> Jonathan
>>>>>
>>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>> ---
>>>>> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 55 +++
>>>>> drivers/iio/Kconfig | 2 +-
>>>>> drivers/iio/Makefile | 1 +
>>>>> drivers/iio/timer/Kconfig | 13 +
>>>>> drivers/iio/timer/Makefile | 1 +
>>>>> drivers/iio/timer/stm32-timer-trigger.c | 466
>+++++++++++++++++++++
>>>>> drivers/iio/trigger/Kconfig | 1 -
>>>>> include/linux/iio/timer/stm32-timer-trigger.h | 62 +++
>>>>> 8 files changed, 599 insertions(+), 2 deletions(-)
>>>>> create mode 100644
>Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>> create mode 100644 drivers/iio/timer/Kconfig
>>>>> create mode 100644 drivers/iio/timer/Makefile
>>>>> create mode 100644 drivers/iio/timer/stm32-timer-trigger.c
>>>>> create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>> new file mode 100644
>>>>> index 0000000..26583dd
>>>>> --- /dev/null
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>>> @@ -0,0 +1,55 @@
>>>>> +What:
>/sys/bus/iio/devices/iio:deviceX/master_mode_available
>>>>> +KernelVersion: 4.10
>>>>> +Contact: benjamin.gaignard@st.com
>>>>> +Description:
>>>>> + Reading returns the list possible master modes which
>are:
>>>>> + - "reset" : The UG bit from the TIMx_EGR
>register is used as trigger output (TRGO).
>>>>> + - "enable" : The Counter Enable signal CNT_EN is
>used as trigger output.
>>>>> + - "update" : The update event is selected as
>trigger output.
>>>>> + For instance a master timer can then
>be used as a prescaler for a slave timer.
>>>>> + - "compare_pulse" : The trigger output send a
>positive pulse when the CC1IF flag is to be set.
>>>>> + - "OC1REF" : OC1REF signal is used as trigger
>output.
>>>>> + - "OC2REF" : OC2REF signal is used as trigger
>output.
>>>>> + - "OC3REF" : OC3REF signal is used as trigger
>output.
>>>>> + - "OC4REF" : OC4REF signal is used as trigger
>output.
>>>>> +
>>>>> +What: /sys/bus/iio/devices/iio:deviceX/master_mode
>>>>> +KernelVersion: 4.10
>>>>> +Contact: benjamin.gaignard@st.com
>>>>> +Description:
>>>>> + Reading returns the current master modes.
>>>>> + Writing set the master mode
>>>>> +
>>>>> +What:
>/sys/bus/iio/devices/iio:deviceX/slave_mode_available
>>>>> +KernelVersion: 4.10
>>>>> +Contact: benjamin.gaignard@st.com
>>>>> +Description:
>>>>> + Reading returns the list possible slave modes which
>are:
>>>>> + - "disabled" : The prescaler is clocked directly by
>the internal clock.
>>>>> + - "encoder_1" : Counter counts up/down on TI2FP1
>edge depending on TI1FP2 level.
>>>>> + - "encoder_2" : Counter counts up/down on TI1FP2
>edge depending on TI2FP1 level.
>>>>> + - "encoder_3" : Counter counts up/down on both
>TI1FP1 and TI2FP2 edges depending
>>>>> + on the level of the other input.
>>>>> + - "reset" : Rising edge of the selected trigger
>input reinitializes the counter
>>>>> + and generates an update of the
>registers.
>>>>> + - "gated" : The counter clock is enabled when
>the trigger input is high.
>>>>> + The counter stops (but is not reset)
>as soon as the trigger becomes low.
>>>>> + Both start and stop of the counter
>are controlled.
>>>>> + - "trigger" : The counter starts at a rising edge
>of the trigger TRGI (but it is not
>>>>> + reset). Only the start of the
>counter is controlled.
>>>>> + - "external_clock": Rising edges of the selected
>trigger (TRGI) clock the counter.
>>>>> +
>>>>> +What: /sys/bus/iio/devices/iio:deviceX/slave_mode
>>>>> +KernelVersion: 4.10
>>>>> +Contact: benjamin.gaignard@st.com
>>>>> +Description:
>>>>> + Reading returns the current slave mode.
>>>>> + Writing set the slave mode
>>>>> +
>>>>> +What:
>/sys/bus/iio/devices/triggerX/sampling_frequency
>>>>> +KernelVersion: 4.10
>>>>> +Contact: benjamin.gaignard@st.com
>>>>> +Description:
>>>>> + Reading returns the current sampling frequency.
>>>>> + Writing an value different of 0 set and start
>sampling.
>>>>> + Writing 0 stop sampling.
>>>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>>>>> index 6743b18..2de2a80 100644
>>>>> --- a/drivers/iio/Kconfig
>>>>> +++ b/drivers/iio/Kconfig
>>>>> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig"
>>>>> source "drivers/iio/pressure/Kconfig"
>>>>> source "drivers/iio/proximity/Kconfig"
>>>>> source "drivers/iio/temperature/Kconfig"
>>>>> -
>>>>> +source "drivers/iio/timer/Kconfig"
>>>>> endif # IIO
>>>>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>>>>> index 87e4c43..b797c08 100644
>>>>> --- a/drivers/iio/Makefile
>>>>> +++ b/drivers/iio/Makefile
>>>>> @@ -32,4 +32,5 @@ obj-y += potentiometer/
>>>>> obj-y += pressure/
>>>>> obj-y += proximity/
>>>>> obj-y += temperature/
>>>>> +obj-y += timer/
>>>>> obj-y += trigger/
>>>>> diff --git a/drivers/iio/timer/Kconfig b/drivers/iio/timer/Kconfig
>>>>> new file mode 100644
>>>>> index 0000000..e3c21f2
>>>>> --- /dev/null
>>>>> +++ b/drivers/iio/timer/Kconfig
>>>>> @@ -0,0 +1,13 @@
>>>>> +#
>>>>> +# Timers drivers
>>>>> +
>>>>> +menu "Timers"
>>>>> +
>>>>> +config IIO_STM32_TIMER_TRIGGER
>>>>> + tristate "STM32 Timer Trigger"
>>>>> + depends on (ARCH_STM32 && OF && MFD_STM32_TIMERS) ||
>COMPILE_TEST
>>>>> + select IIO_TRIGGERED_EVENT
>>>>> + help
>>>>> + Select this option to enable STM32 Timer Trigger
>>>>> +
>>>>> +endmenu
>>>>> diff --git a/drivers/iio/timer/Makefile
>b/drivers/iio/timer/Makefile
>>>>> new file mode 100644
>>>>> index 0000000..4ad95ec9
>>>>> --- /dev/null
>>>>> +++ b/drivers/iio/timer/Makefile
>>>>> @@ -0,0 +1 @@
>>>>> +obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o
>>>>> diff --git a/drivers/iio/timer/stm32-timer-trigger.c
>b/drivers/iio/timer/stm32-timer-trigger.c
>>>>> new file mode 100644
>>>>> index 0000000..8d16e8f
>>>>> --- /dev/null
>>>>> +++ b/drivers/iio/timer/stm32-timer-trigger.c
>>>>> @@ -0,0 +1,466 @@
>>>>> +/*
>>>>> + * Copyright (C) STMicroelectronics 2016
>>>>> + *
>>>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>> + *
>>>>> + * License terms: GNU General Public License (GPL), version 2
>>>>> + */
>>>>> +
>>>>> +#include <linux/iio/iio.h>
>>>>> +#include <linux/iio/sysfs.h>
>>>>> +#include <linux/iio/timer/stm32-timer-trigger.h>
>>>>> +#include <linux/iio/trigger.h>
>>>>> +#include <linux/iio/triggered_event.h>
>>>>> +#include <linux/interrupt.h>
>>>>> +#include <linux/mfd/stm32-timers.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +
>>>>> +#define MAX_TRIGGERS 6
>>>>> +#define MAX_VALIDS 5
>>>>> +
>>>>> +/* List the triggers created by each timer */
>>>>> +static const void *triggers_table[][MAX_TRIGGERS] = {
>>>>> + { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
>>>>> + { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
>>>>> + { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
>>>>> + { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
>>>>> + { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
>>>>> + { TIM6_TRGO,},
>>>>> + { TIM7_TRGO,},
>>>>> + { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
>>>>> + { TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
>>>>> + { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
>>>>> +};
>>>>> +
>>>>> +/* List the triggers accepted by each timer */
>>>>> +static const void *valids_table[][MAX_VALIDS] = {
>>>>> + { TIM5_TRGO, TIM2_TRGO, TIM4_TRGO, TIM3_TRGO,},
>>>>> + { TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
>>>>> + { TIM1_TRGO, TIM8_TRGO, TIM5_TRGO, TIM4_TRGO,},
>>>>> + { TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
>>>>> + { TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
>>>>> + { }, /* timer 6 */
>>>>> + { }, /* timer 7 */
>>>>> + { TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
>>>>> + { TIM2_TRGO, TIM3_TRGO,},
>>>>> + { TIM4_TRGO, TIM5_TRGO,},
>>>>> +};
>>>>> +
>>>>> +struct stm32_timer_trigger {
>>>>> + struct device *dev;
>>>>> + struct regmap *regmap;
>>>>> + struct clk *clk;
>>>>> + u32 max_arr;
>>>>> + const void *triggers;
>>>>> + const void *valids;
>>>>> +};
>>>>> +
>>>>> +static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>>>> + unsigned int frequency)
>>>>> +{
>>>>> + unsigned long long prd, div;
>>>>> + int prescaler = 0;
>>>>> + u32 ccer, cr1;
>>>>> +
>>>>> + /* Period and prescaler values depends of clock rate */
>>>>> + div = (unsigned long long)clk_get_rate(priv->clk);
>>>>> +
>>>>> + do_div(div, frequency);
>>>>> +
>>>>> + prd = div;
>>>>> +
>>>>> + /*
>>>>> + * Increase prescaler value until we get a result that fit
>>>>> + * with auto reload register maximum value.
>>>>> + */
>>>>> + while (div > priv->max_arr) {
>>>>> + prescaler++;
>>>>> + div = prd;
>>>>> + do_div(div, (prescaler + 1));
>>>>> + }
>>>>> + prd = div;
>>>>> +
>>>>> + if (prescaler > MAX_TIM_PSC) {
>>>>> + dev_err(priv->dev, "prescaler exceeds the maximum
>value\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + /* Check if nobody else use the timer */
>>>>> + regmap_read(priv->regmap, TIM_CCER, &ccer);
>>>>> + if (ccer & TIM_CCER_CCXE)
>>>>> + return -EBUSY;
>>>>> +
>>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>>> + if (!(cr1 & TIM_CR1_CEN))
>>>>> + clk_enable(priv->clk);
>>>>> +
>>>>> + regmap_write(priv->regmap, TIM_PSC, prescaler);
>>>>> + regmap_write(priv->regmap, TIM_ARR, prd - 1);
>>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE,
>TIM_CR1_ARPE);
>>>>> +
>>>>> + /* Force master mode to update mode */
>>>>> + regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS,
>0x20);
>>>>> +
>>>>> + /* Make sure that registers are updated */
>>>>> + regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG,
>TIM_EGR_UG);
>>>>> +
>>>>> + /* Enable controller */
>>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
>TIM_CR1_CEN);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void stm32_timer_stop(struct stm32_timer_trigger *priv)
>>>>> +{
>>>>> + u32 ccer, cr1;
>>>>> +
>>>>> + regmap_read(priv->regmap, TIM_CCER, &ccer);
>>>>> + if (ccer & TIM_CCER_CCXE)
>>>>> + return;
>>>>> +
>>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>>> + if (cr1 & TIM_CR1_CEN)
>>>>> + clk_disable(priv->clk);
>>>>> +
>>>>> + /* Stop timer */
>>>>> + regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
>>>>> + regmap_write(priv->regmap, TIM_PSC, 0);
>>>>> + regmap_write(priv->regmap, TIM_ARR, 0);
>>>>> +}
>>>>> +
>>>>> +static ssize_t stm32_tt_store_frequency(struct device *dev,
>>>>> + struct device_attribute
>*attr,
>>>>> + const char *buf, size_t len)
>>>>> +{
>>>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>>>> + struct stm32_timer_trigger *priv =
>iio_trigger_get_drvdata(trig);
>>>>> + unsigned int freq;
>>>>> + int ret;
>>>>> +
>>>>> + ret = kstrtouint(buf, 10, &freq);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + if (freq == 0) {
>>>>> + stm32_timer_stop(priv);
>>>>> + } else {
>>>>> + ret = stm32_timer_start(priv, freq);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + return len;
>>>>> +}
>>>>> +
>>>>> +static ssize_t stm32_tt_read_frequency(struct device *dev,
>>>>> + struct device_attribute
>*attr, char *buf)
>>>>> +{
>>>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>>>> + struct stm32_timer_trigger *priv =
>iio_trigger_get_drvdata(trig);
>>>>> + u32 psc, arr, cr1;
>>>>> + unsigned long long freq = 0;
>>>>> +
>>>>> + regmap_read(priv->regmap, TIM_CR1, &cr1);
>>>>> + regmap_read(priv->regmap, TIM_PSC, &psc);
>>>>> + regmap_read(priv->regmap, TIM_ARR, &arr);
>>>>> +
>>>>> + if (psc && arr && (cr1 & TIM_CR1_CEN)) {
>>>>> + freq = (unsigned long long)clk_get_rate(priv->clk);
>>>>> + do_div(freq, psc);
>>>>> + do_div(freq, arr);
>>>>> + }
>>>>> +
>>>>> + return sprintf(buf, "%d\n", (unsigned int)freq);
>>>>> +}
>>>>> +
>>>>> +static IIO_DEV_ATTR_SAMP_FREQ(0660,
>>>>> + stm32_tt_read_frequency,
>>>>> + stm32_tt_store_frequency);
>>>>> +
>>>>> +static struct attribute *stm32_trigger_attrs[] = {
>>>>> + &iio_dev_attr_sampling_frequency.dev_attr.attr,
>>>>> + NULL,
>>>>> +};
>>>>> +
>>>>> +static const struct attribute_group stm32_trigger_attr_group = {
>>>>> + .attrs = stm32_trigger_attrs,
>>>>> +};
>>>>> +
>>>>> +static const struct attribute_group *stm32_trigger_attr_groups[]
>= {
>>>>> + &stm32_trigger_attr_group,
>>>>> + NULL,
>>>>> +};
>>>>> +
>>>>> +static char *master_mode_table[] = {
>>>>> + "reset",
>>>>> + "enable",
>>>>> + "update",
>>>>> + "compare_pulse",
>>>>> + "OC1REF",
>>>>> + "OC2REF",
>>>>> + "OC3REF",
>>>>> + "OC4REF"
>>>>> +};
>>>>> +
>>>>> +static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>>>> + struct device_attribute
>*attr,
>>>>> + char *buf)
>>>>> +{
>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> + u32 cr2;
>>>>> +
>>>>> + regmap_read(priv->regmap, TIM_CR2, &cr2);
>>>>> + cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
>>>>> +
>>>>> + return snprintf(buf, PAGE_SIZE, "%s\n",
>master_mode_table[cr2]);
>>>>> +}
>>>>> +
>>>>> +static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>>>> + struct device_attribute
>*attr,
>>>>> + const char *buf, size_t
>len)
>>>>> +{
>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>>>> + if (!strncmp(master_mode_table[i], buf,
>>>>> + strlen(master_mode_table[i]))) {
>>>>> + regmap_update_bits(priv->regmap, TIM_CR2,
>>>>> + TIM_CR2_MMS, i <<
>TIM_CR2_MMS_SHIFT);
>>>>> + return len;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static IIO_CONST_ATTR(master_mode_available,
>>>>> + "reset enable update compare_pulse OC1REF OC2REF OC3REF
>OC4REF");
>>>>> +
>>>>> +static IIO_DEVICE_ATTR(master_mode, 0660,
>>>>> + stm32_tt_show_master_mode,
>>>>> + stm32_tt_store_master_mode,
>>>>> + 0);
>>>>> +
>>>>> +static char *slave_mode_table[] = {
>>>>> + "disabled",
>>>>> + "encoder_1",
>>>>> + "encoder_2",
>>>>> + "encoder_3",
>>>>> + "reset",
>>>>> + "gated",
>>>>> + "trigger",
>>>>> + "external_clock",
>>>>> +};
>>>>> +
>>>>> +static ssize_t stm32_tt_show_slave_mode(struct device *dev,
>>>>> + struct device_attribute
>*attr,
>>>>> + char *buf)
>>>>> +{
>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> + u32 smcr;
>>>>> +
>>>>> + regmap_read(priv->regmap, TIM_SMCR, &smcr);
>>>>> + smcr &= TIM_SMCR_SMS;
>>>>> +
>>>>> + return snprintf(buf, PAGE_SIZE, "%s\n",
>slave_mode_table[smcr]);
>>>>> +}
>>>>> +
>>>>> +static ssize_t stm32_tt_store_slave_mode(struct device *dev,
>>>>> + struct device_attribute
>*attr,
>>>>> + const char *buf, size_t
>len)
>>>>> +{
>>>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < ARRAY_SIZE(slave_mode_table); i++) {
>>>>> + if (!strncmp(slave_mode_table[i], buf,
>>>>> + strlen(slave_mode_table[i]))) {
>>>>> + regmap_update_bits(priv->regmap,
>>>>> + TIM_SMCR, TIM_SMCR_SMS,
>i);
>>>>> + return len;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static IIO_CONST_ATTR(slave_mode_available,
>>>>> +"disabled encoder_1 encoder_2 encoder_3 reset gated trigger
>external_clock");
>>>>> +
>>>>> +static IIO_DEVICE_ATTR(slave_mode, 0660,
>>>>> + stm32_tt_show_slave_mode,
>>>>> + stm32_tt_store_slave_mode,
>>>>> + 0);
>>>>> +
>>>>> +static struct attribute *stm32_timer_attrs[] = {
>>>>> + &iio_dev_attr_master_mode.dev_attr.attr,
>>>>> + &iio_const_attr_master_mode_available.dev_attr.attr,
>>>>> + &iio_dev_attr_slave_mode.dev_attr.attr,
>>>>> + &iio_const_attr_slave_mode_available.dev_attr.attr,
>>>>> + NULL,
>>>>> +};
>>>>> +
>>>>> +static const struct attribute_group stm32_timer_attr_group = {
>>>>> + .attrs = stm32_timer_attrs,
>>>>> +};
>>>>> +
>>>>> +static const struct iio_trigger_ops timer_trigger_ops = {
>>>>> + .owner = THIS_MODULE,
>>>>> +};
>>>>> +
>>>>> +static int stm32_setup_iio_triggers(struct stm32_timer_trigger
>*priv)
>>>>> +{
>>>>> + int ret;
>>>>> + const char * const *cur = priv->triggers;
>>>>> +
>>>>> + while (cur && *cur) {
>>>>> + struct iio_trigger *trig;
>>>>> +
>>>>> + trig = devm_iio_trigger_alloc(priv->dev, "%s",
>*cur);
>>>>> + if (!trig)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + trig->dev.parent = priv->dev->parent;
>>>>> + trig->ops = &timer_trigger_ops;
>>>>> + trig->dev.groups = stm32_trigger_attr_groups;
>>>>> + iio_trigger_set_drvdata(trig, priv);
>>>>> +
>>>>> + ret = devm_iio_trigger_register(priv->dev, trig);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + cur++;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * is_stm32_timer_trigger
>>>>> + * @trig: trigger to be checked
>>>>> + *
>>>>> + * return true if the trigger is a valid stm32 iio timer trigger
>>>>> + * either return false
>>>>> + */
>>>>> +bool is_stm32_timer_trigger(struct iio_trigger *trig)
>>>>> +{
>>>>> + return (trig->ops == &timer_trigger_ops);
>>>>> +}
>>>>> +EXPORT_SYMBOL(is_stm32_timer_trigger);
>>>>> +
>>>>> +static int stm32_validate_trigger(struct iio_dev *indio_dev,
>>>>> + struct iio_trigger *trig)
>>>>> +{
>>>>> + struct stm32_timer_trigger *priv = iio_priv(indio_dev);
>>>>> + const char * const *cur = priv->valids;
>>>>> + unsigned int i = 0;
>>>>> +
>>>>> + if (!is_stm32_timer_trigger(trig))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + while (cur && *cur) {
>>>>> + if (!strncmp(trig->name, *cur, strlen(trig->name)))
>{
>>>>> + regmap_update_bits(priv->regmap,
>>>>> + TIM_SMCR, TIM_SMCR_TS,
>>>>> + i << TIM_SMCR_TS_SHIFT);
>>>>> + return 0;
>>>>> + }
>>>>> + cur++;
>>>>> + i++;
>>>>> + }
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static const struct iio_info stm32_trigger_info = {
>>>>> + .driver_module = THIS_MODULE,
>>>>> + .validate_trigger = stm32_validate_trigger,
>>>>> + .attrs = &stm32_timer_attr_group,
>>>>> +};
>>>>> +
>>>>> +static struct stm32_timer_trigger *stm32_setup_iio_device(struct
>device *dev)
>>>>> +{
>>>>> + struct iio_dev *indio_dev;
>>>>> + int ret;
>>>>> +
>>>>> + indio_dev = devm_iio_device_alloc(dev,
>>>>> + sizeof(struct
>stm32_timer_trigger));
>>>>> + if (!indio_dev)
>>>>> + return NULL;
>>>>> +
>>>>> + indio_dev->name = dev_name(dev);
>>>>> + indio_dev->dev.parent = dev;
>>>>> + indio_dev->info = &stm32_trigger_info;
>>>>> + indio_dev->modes = INDIO_EVENT_TRIGGERED;
>>>>> + indio_dev->num_channels = 0;
>>>>> + indio_dev->dev.of_node = dev->of_node;
>>>>> +
>>>>> + ret = devm_iio_device_register(dev, indio_dev);
>>>>> + if (ret)
>>>>> + return NULL;
>>>>> +
>>>>> + return iio_priv(indio_dev);
>>>>> +}
>>>>> +
>>>>> +static int stm32_timer_trigger_probe(struct platform_device
>*pdev)
>>>>> +{
>>>>> + struct device *dev = &pdev->dev;
>>>>> + struct stm32_timer_trigger *priv;
>>>>> + struct stm32_timers *ddata =
>dev_get_drvdata(pdev->dev.parent);
>>>>> + unsigned int index;
>>>>> + int ret;
>>>>> +
>>>>> + if (of_property_read_u32(dev->of_node, "reg", &index))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + if (index >= ARRAY_SIZE(triggers_table))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + /* Create an IIO device only if we have triggers to be
>validated */
>>>>> + if (*valids_table[index])
>>>>> + priv = stm32_setup_iio_device(dev);
>>>>
>>>> I still don't like this. Really feels like we shouldn't be creating
>an
>>>> iio device with all the bagage that carries just to allow us to do
>the
>>>> trigger trees. We ought to have a much more light weight solution
>for this
>>>> functionality - we aren't typically even using the interrupt tree
>stuff
>>>> that the triggers for devices are all really about.
>>>>
>>>> A simpler approach of allowing each trigger the option of a parent
>seems like
>>>> it would be cleaner. Could be done entirely within this driver in
>the first
>>>> instance. Basically it would just look like your master and slave
>attributes
>>>> but have those under triggerX not iio:deviceX.
>>>>
>>>> We can work out how to make it more generic later - including
>perhaps the
>>>> option to trigger from triggers outside this driver, using some
>parallel
>>>> infrastructure to the device triggering.
>>>>
>>>>
>>>>> + else
>>>>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>>>> +
>>>>> + if (!priv)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + priv->dev = dev;
>>>>> + priv->regmap = ddata->regmap;
>>>>> + priv->clk = ddata->clk;
>>>>> + priv->max_arr = ddata->max_arr;
>>>>> + priv->triggers = triggers_table[index];
>>>>> + priv->valids = valids_table[index];
>>>>> +
>>>>> + ret = stm32_setup_iio_triggers(priv);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + platform_set_drvdata(pdev, priv);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id stm32_trig_of_match[] = {
>>>>> + { .compatible = "st,stm32-timer-trigger", },
>>>>> + { /* end node */ },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
>>>>> +
>>>>> +static struct platform_driver stm32_timer_trigger_driver = {
>>>>> + .probe = stm32_timer_trigger_probe,
>>>>> + .driver = {
>>>>> + .name = "stm32-timer-trigger",
>>>>> + .of_match_table = stm32_trig_of_match,
>>>>> + },
>>>>> +};
>>>>> +module_platform_driver(stm32_timer_trigger_driver);
>>>>> +
>>>>> +MODULE_ALIAS("platform: stm32-timer-trigger");
>>>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer Trigger
>driver");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>> diff --git a/drivers/iio/trigger/Kconfig
>b/drivers/iio/trigger/Kconfig
>>>>> index 809b2e7..f2af4fe 100644
>>>>> --- a/drivers/iio/trigger/Kconfig
>>>>> +++ b/drivers/iio/trigger/Kconfig
>>>>> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER
>>>>>
>>>>> To compile this driver as a module, choose M here: the
>>>>> module will be called iio-trig-sysfs.
>>>>> -
>>>> Clean this up.
>>>
>>> ok
>>>
>>>>> endmenu
>>>>> diff --git a/include/linux/iio/timer/stm32-timer-trigger.h
>b/include/linux/iio/timer/stm32-timer-trigger.h
>>>>> new file mode 100644
>>>>> index 0000000..55535ae
>>>>> --- /dev/null
>>>>> +++ b/include/linux/iio/timer/stm32-timer-trigger.h
>>>>> @@ -0,0 +1,62 @@
>>>>> +/*
>>>>> + * Copyright (C) STMicroelectronics 2016
>>>>> + *
>>>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>>> + *
>>>>> + * License terms: GNU General Public License (GPL), version 2
>>>>> + */
>>>>> +
>>>>> +#ifndef _STM32_TIMER_TRIGGER_H_
>>>>> +#define _STM32_TIMER_TRIGGER_H_
>>>>> +
>>>>> +#define TIM1_TRGO "tim1_trgo"
>>>>> +#define TIM1_CH1 "tim1_ch1"
>>>>> +#define TIM1_CH2 "tim1_ch2"
>>>>> +#define TIM1_CH3 "tim1_ch3"
>>>>> +#define TIM1_CH4 "tim1_ch4"
>>>>> +
>>>>> +#define TIM2_TRGO "tim2_trgo"
>>>>> +#define TIM2_CH1 "tim2_ch1"
>>>>> +#define TIM2_CH2 "tim2_ch2"
>>>>> +#define TIM2_CH3 "tim2_ch3"
>>>>> +#define TIM2_CH4 "tim2_ch4"
>>>>> +
>>>>> +#define TIM3_TRGO "tim3_trgo"
>>>>> +#define TIM3_CH1 "tim3_ch1"
>>>>> +#define TIM3_CH2 "tim3_ch2"
>>>>> +#define TIM3_CH3 "tim3_ch3"
>>>>> +#define TIM3_CH4 "tim3_ch4"
>>>>> +
>>>>> +#define TIM4_TRGO "tim4_trgo"
>>>>> +#define TIM4_CH1 "tim4_ch1"
>>>>> +#define TIM4_CH2 "tim4_ch2"
>>>>> +#define TIM4_CH3 "tim4_ch3"
>>>>> +#define TIM4_CH4 "tim4_ch4"
>>>>> +
>>>>> +#define TIM5_TRGO "tim5_trgo"
>>>>> +#define TIM5_CH1 "tim5_ch1"
>>>>> +#define TIM5_CH2 "tim5_ch2"
>>>>> +#define TIM5_CH3 "tim5_ch3"
>>>>> +#define TIM5_CH4 "tim5_ch4"
>>>>> +
>>>>> +#define TIM6_TRGO "tim6_trgo"
>>>>> +
>>>>> +#define TIM7_TRGO "tim7_trgo"
>>>>> +
>>>>> +#define TIM8_TRGO "tim8_trgo"
>>>>> +#define TIM8_CH1 "tim8_ch1"
>>>>> +#define TIM8_CH2 "tim8_ch2"
>>>>> +#define TIM8_CH3 "tim8_ch3"
>>>>> +#define TIM8_CH4 "tim8_ch4"
>>>>> +
>>>>> +#define TIM9_TRGO "tim9_trgo"
>>>>> +#define TIM9_CH1 "tim9_ch1"
>>>>> +#define TIM9_CH2 "tim9_ch2"
>>>>> +
>>>>> +#define TIM12_TRGO "tim12_trgo"
>>>>> +#define TIM12_CH1 "tim12_ch1"
>>>>> +#define TIM12_CH2 "tim12_ch2"
>>>>> +
>>>>> +bool is_stm32_timer_trigger(struct iio_trigger *trig);
>>>>> +
>>>>> +#endif
>>>>>
>>>>
>>>
>>>
>>>
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH 6/9] arm64: dts: ls1046a: add MSI dts node
From: Rob Herring @ 2017-01-03 17:12 UTC (permalink / raw)
To: Minghuan Lian
Cc: devicetree, Jason Cooper, Roy Zang, Marc Zyngier, linux-kernel,
Stuart Yoder, Mingkai Hu, Scott Wood, Yang-Leo Li,
linux-arm-kernel
In-Reply-To: <1482829985-24421-6-git-send-email-Minghuan.Lian@nxp.com>
On Tue, Dec 27, 2016 at 05:13:02PM +0800, Minghuan Lian wrote:
> LS1046a includes 3 MSI controllers.
> Each controller supports 128 interrupts.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> .../interrupt-controller/fsl,ls-scfg-msi.txt | 1 +
> arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 31 ++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> index 2755cd1..54597b0 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt
> @@ -6,6 +6,7 @@ Required properties:
> Layerscape PCIe MSI controller block such as:
> "fsl,ls1021a-msi"
> "fsl,ls1043a-msi"
> + "fsl,ls1046a-msi"
Differing whitespace.
Otherwise,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/9] irqchip/ls-scfg-msi: fix typo of MSI compatible strings
From: Rob Herring @ 2017-01-03 17:11 UTC (permalink / raw)
To: Minghuan Lian
Cc: devicetree, Jason Cooper, Roy Zang, Marc Zyngier, linux-kernel,
Stuart Yoder, Mingkai Hu, Scott Wood, Yang-Leo Li,
linux-arm-kernel
In-Reply-To: <1482829985-24421-1-git-send-email-Minghuan.Lian@nxp.com>
On Tue, Dec 27, 2016 at 05:12:57PM +0800, Minghuan Lian wrote:
> The patch is to fix typo of the Layerscape SCFG MSI dts compatible
> strings. "1" is replaced by "l".
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
> ---
> .../devicetree/bindings/interrupt-controller/fsl,ls-scfg-msi.txt | 6 +++---
> drivers/irqchip/irq-ls-scfg-msi.c | 6 ++++--
> 2 files changed, 7 insertions(+), 5 deletions(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 2/3] DT: bingdings: power: reset: add linkstation-reset doc
From: Rob Herring @ 2017-01-03 17:09 UTC (permalink / raw)
To: Roger Shimizu
Cc: Mark Rutland, Andrew Lunn, Ryan Tandy, devicetree, linux-pm,
Sebastian Reichel, linux-arm-kernel
In-Reply-To: <20161227070611.14852-3-rogershimizu@gmail.com>
On Tue, Dec 27, 2016 at 04:06:10PM +0900, Roger Shimizu wrote:
> Add linkstation-reset doc to describe the newly added
> POWER_RESET_LINKSTATION driver, which controls magic command
> sending to UART1 to power-off Buffalo Linkstation / KuroBox
> and their variants.
>
> To: Sebastian Reichel <sre@kernel.org>
> To: Rob Herring <robh+dt@kernel.org>
> To: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Ryan Tandy <ryan@nardis.ca>
> Cc: linux-pm@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
>
> Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
> ---
> .../bindings/power/reset/linkstation-reset.txt | 26 ++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> new file mode 100644
> index 000000000000..815e340318f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> @@ -0,0 +1,26 @@
> +* Buffalo Linkstation Reset Driver
> +
> +Power of some Buffalo Linkstation or KuroBox Pro is managed by
> +micro-controller, which connects to UART1. After being fed from UART1
> +by a few magic numbers, the so-called power-off command,
> +the micro-controller will turn power off the device.
This needs to model the uC connected to the UART rather than some node
that defines only some portion of the functionality. I'm working on
bindings and proper bus support for this[1], but it's not done yet.
Though, the binding side is pretty simple.
Rob
[1] https://git.kernel.org/cgit/linux/kernel/git/robh/linux.git/log/?h=serial-bus-v2
^ permalink raw reply
* Re: [PATCH 2/4] input: tm2-touchkey: Add touchkey driver support for TM2
From: Krzysztof Kozlowski @ 2017-01-03 16:59 UTC (permalink / raw)
To: Jaechul Lee
Cc: Mark Rutland, devicetree, linux-samsung-soc, Andi Shyti,
Chanwoo Choi, Catalin Marinas, Dmitry Torokhov, Will Deacon,
linux-kernel, Rob Herring, Javier Martinez Canillas, Kukjin Kim,
Krzysztof Kozlowski, linux-input, galaxyra, beomho.seo,
linux-arm-kernel
In-Reply-To: <1483430237-26823-3-git-send-email-jcsing.lee@samsung.com>
On Tue, Jan 03, 2017 at 04:57:15PM +0900, Jaechul Lee wrote:
> This patch adds support for the TM2 touch key and led
> functionlity.
>
> The driver interfaces with userspace through an input device and
> reports KEY_PHONE and KEY_BACK event types. LED brightness can be
> controlled by "/sys/class/leds/tm2-touchkey/brightness".
>
> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> ---
> drivers/input/keyboard/Kconfig | 11 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/tm2-touchkey.c | 326 ++++++++++++++++++++++++++++++++++
> 3 files changed, 338 insertions(+)
> create mode 100644 drivers/input/keyboard/tm2-touchkey.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index cbd75cf..72c0ba1 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -666,6 +666,17 @@ config KEYBOARD_TC3589X
> To compile this driver as a module, choose M here: the
> module will be called tc3589x-keypad.
>
> +config KEYBOARD_TM2_TOUCHKEY
> + tristate "tm2-touchkey support"
> + depends on I2C
> + help
> + Say Y here to enable the tm2-touchkey.
The ending full stop is not needed.
> + touchkey driver for tm2. This driver can enable
> + the interrupt and make input events and control led brightness.
This sentence needs improvements. How about just:
"Beside input device, this driver provides also brightness control for
LEDs on touch keys."
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/5] devicetree: mfd: Add binding for the TI LM3533
From: Rob Herring @ 2017-01-03 16:56 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Mark Rutland, Jonathan Cameron, Lee Jones, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Richard Purdie,
Jacek Anaszewski, Pavel Machek, Jingoo Han, devicetree,
linux-kernel, linux-iio, linux-leds, Bjorn Andersson
In-Reply-To: <20161226181153.11271-1-bjorn.andersson@linaro.org>
On Mon, Dec 26, 2016 at 10:11:49AM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>
> Add the binding for the Texas Instruments LM3533 lighting power
> solution.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> I had Acks from Jonathan and Rob on v3, but dropped these due to the added
> compatible properties.
>
> Changes since v3:
> - Added compatible to sub-nodes, per Lee's requested to treat them as separate
> pieces.
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RESEND 1/2] document: dt: add binding for Hi3660 SoC
From: Rob Herring @ 2017-01-03 16:52 UTC (permalink / raw)
To: Chen Feng
Cc: mark.rutland, devicetree, xuyiping, catalin.marinas, suzhuangluan,
will.deacon, linux-kernel, xuwei5, linux-arm-kernel
In-Reply-To: <1482744972-56622-1-git-send-email-puck.chen@hisilicon.com>
On Mon, Dec 26, 2016 at 05:36:11PM +0800, Chen Feng wrote:
> Add binding for hisilicon Hi3660 SoC and HiKey960 Board.
>
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> ---
> Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt | 4 ++++
> 1 file changed, 4 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH RESEND] dmaengine: stm32-dma: Rework starting transfer management
From: M'boumba Cedric Madianga @ 2017-01-03 16:51 UTC (permalink / raw)
To: Vinod Koul
Cc: Mark Rutland, devicetree, Alexandre Torgue, linux-kernel,
Rob Herring, Maxime Coquelin, dmaengine, dan.j.williams,
linux-arm-kernel
In-Reply-To: <20170103035703.GE3573@localhost>
Hi Vinod,
> ah sorry, i missed to push topic/stm32-dma. can you rebase onnthis and
> resend again. Btw this fails as as well..
I have tried again and I don't have any issue.
Please see below my way of working and let me know if there is
something I don't understand:
slave-dma git://git.infradead.org/users/vkoul/slave-dma.git
$ git fetch slave-dma
$ git checkout -b dma-fixes slave-dma/topic/dma-fixes
$ git checkout -b dma-stm32 slave-dma/topic/stm32-dma
$ git rebase dma-fixes
First, rewinding head to replay your work on top of it...
Applying: dmaengine: stm32-dma: Fix typo in Kconfig
Applying: dt-bindings: stm32-dma: Fix typo regarding DMA client binding
Applying: dmaengine: stm32-dma: Rework starting transfer management
Applying: dmaengine: stm32-dma: Fix residue computation issue in cyclic mode
Applying: dmaengine: stm32-dma: Add synchronization support
Applying: dmaengine: stm32-dma: Add max_burst support
Best regards,
Cedric
^ permalink raw reply
* Re: [PATCH 4/4] arm64: dts: exynos: Add tm2 touchkey node
From: Krzysztof Kozlowski @ 2017-01-03 16:49 UTC (permalink / raw)
To: Jaechul Lee
Cc: Mark Rutland, devicetree, linux-samsung-soc, Andi Shyti,
Chanwoo Choi, Catalin Marinas, Dmitry Torokhov, Will Deacon,
linux-kernel, Rob Herring, Javier Martinez Canillas, Kukjin Kim,
Krzysztof Kozlowski, linux-input, galaxyra, beomho.seo,
linux-arm-kernel
In-Reply-To: <1483430237-26823-5-git-send-email-jcsing.lee@samsung.com>
On Tue, Jan 03, 2017 at 04:57:17PM +0900, Jaechul Lee wrote:
> Add DT node support for TM2 touchkey device.
>
> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
The order of sign-offs should follow the order of "who touched it". I
assume that the real order was like this:
1. Initially create by Beomho,
2. Adjustments by Andi,
3. Final adjustments and sending by you.
If that is correct then the order should be 1->2->3.
Beside of that, patch looks good.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/3] devicetree: bq27425: add documentation for bq27425 fuel gauge
From: Rob Herring @ 2017-01-03 16:47 UTC (permalink / raw)
To: Matt Ranostay
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Sebastian Reichel, Tony Lindgren
In-Reply-To: <20161226054442.23599-3-matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
On Sun, Dec 25, 2016 at 09:44:41PM -0800, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> ---
> .../devicetree/bindings/power/bq27425.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/bq27425.txt
>
> diff --git a/Documentation/devicetree/bindings/power/bq27425.txt b/Documentation/devicetree/bindings/power/bq27425.txt
> new file mode 100644
> index 000000000000..f09f787a9378
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/bq27425.txt
> @@ -0,0 +1,25 @@
> +* TI BQ27425 Fuel Gauge
> +
> +http://www.ti.com/lit/ds/symlink/bq27425-g2a.pdf
> +
> +Please note that if any of the optional properties are defined
> +then all settings must be.
> +
> +Required properties:
> +- compatible: Should be "ti,bq27425"
> +- reg: integer, i2c of the device
s/i2c/I2C address/
> +
> +Optional properties:
> +- ti,design-capacity: integer of mAh of the battery
> +- ti,design-energy: integer of the mWh of the battery
> +- ti,terminate-voltage: integer of mV of the dead voltage of
> + the battery
These all need unit suffixes. mAh and mWh may need to be added to
property-units.txt.
> +
> +bq27425 {
> + compatible = "ti,bq27425";
> + reg = <0x55>;
> +
> + ti,design-capacity = <1360>;
> + ti,design-energy = <4970>;
> + ti,terminate-voltage = <3200>;
> +};
> --
> 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
--
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/4] arm64: dts: exynos: make tm2 and tm2e independent from each other
From: Krzysztof Kozlowski @ 2017-01-03 16:41 UTC (permalink / raw)
To: Andi Shyti
Cc: Mark Rutland, devicetree, linux-samsung-soc, Dmitry Torokhov,
Chanwoo Choi, Catalin Marinas, Jaechul Lee, Will Deacon,
linux-kernel, Krzysztof Kozlowski, Javier Martinez Canillas,
Rob Herring, Kukjin Kim, linux-input, galaxyra, beomho.seo,
linux-arm-kernel
In-Reply-To: <20170103102548.73jg6qddlcthe2mu@gangnam.samsung>
On Tue, Jan 03, 2017 at 07:25:48PM +0900, Andi Shyti wrote:
> > >> > Currently tm2e dts includes tm2 but there are some differences
> > >> > between the two boards and tm2 has some properties that tm2e
> > >> > doesn't have.
> > >> >
> > >> > That's why it's important to keep the two dts files independent
> > >> > and put all the commonalities in a tm2-common.dtsi file.
> > >> >
> > >> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > >> > Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> > >> > ---
> > >> > .../boot/dts/exynos/exynos5433-tm2-common.dtsi | 1046 ++++++++++++++++++++
> > >> > arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 1033 +------------------
> > >> > arch/arm64/boot/dts/exynos/exynos5433-tm2e.dts | 2 +-
> > >> > 3 files changed, 1049 insertions(+), 1032 deletions(-)
> > >> > create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> > >>
> > >> I would like to see here the rename and diff from it. Not entire delta
> > >> (deletions and addons). It is not possible to compare it... I think
> > >> git supports it by default with similarity of 50%.
> > >
> > > I understand, it's indeed quite cryptic to understand. But all
> > > the diff algorithms (patience, minimal, histogram, myers) give
> > > the same result. I don't know how to make it better.
> > >
> > > I could split this patch, but this also means breaking tm2's
> > > functionality, which looks worse.
> > >
> > > Please tell me if you know a better way for generating the patch.
> >
> > git format-patch -M95%?
>
> Same thing with all M values.
>
> Because exynos5433-tm2.dts results modified, while
> exynos5433-tm2-common.dtsi is new. Even though I did:
>
> 1. mv exynos5433-tm2.dts exynos5433-tm2-common.dtsi
> 2. copied pieces from exynos5433-tm2-common.dtsi to a new
> exynos5433-tm2.dts
mv/etc won't help. You need to convince git format-patch that it is
rename. For that, you need -B so copy will be considered a rename
(otherwise rename will be detected only if a file is removed), so:
git format-patch -B50% -1
Please resend with this approach.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/8] ARM: dts: armada-370-rd: Utilize new DSA binding
From: Andrew Lunn @ 2017-01-03 16:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jason Cooper, vivien.didelot, Russell King, open list,
Rob Herring, Gregory Clement, linux-arm-kernel,
Sebastian Hesselbarth
In-Reply-To: <20170102022249.10657-2-f.fainelli@gmail.com>
> +
> + switch: switch@10 {
> + compatible = "marvell,mv88e6085";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <16>;
Hummm, a device tree question. switch@10, reg = <16>. Is there an
implicit understanding that the 10 is hex?
Andrew
^ permalink raw reply
* Re: [PATCHv3] mfd: cpcap: Add minimal support
From: Lee Jones @ 2017-01-03 16:35 UTC (permalink / raw)
To: Tony Lindgren
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott
In-Reply-To: <20161206004856.11996-1-tony@atomide.com>
On Mon, 05 Dec 2016, Tony Lindgren wrote:
> Many Motorola phones like droid 4 are using a custom PMIC called CPCAP
> or 6556002. We can support it's core features quite easily with regmap_spi
> and regmap_irq.
>
> The children of cpcap, such as regulators, ADC and USB, can be just regular
> device drivers and defined in the dts file. They get probed as we call
> of_platform_populate() at the end of our probe, and then the children
> can just call dev_get_regmap(dev.parent, NULL) to get the regmap.
>
> Cc: devicetree@vger.kernel.org
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Michael Scott <michael.scott@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>
> Changes from v2:
>
> - Fix typos in binding documentation for #size-cells and spi-cs-high
> - Add ack from Rob
>
> ---
> .../devicetree/bindings/mfd/motorola-cpcap.txt | 31 +++
> drivers/mfd/Kconfig | 11 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/motorola-cpcap.c | 244 +++++++++++++++++
> include/linux/mfd/motorola-cpcap.h | 289 +++++++++++++++++++++
> 5 files changed, 576 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/motorola-cpcap.txt
> create mode 100644 drivers/mfd/motorola-cpcap.c
> create mode 100644 include/linux/mfd/motorola-cpcap.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/motorola-cpcap.txt b/Documentation/devicetree/bindings/mfd/motorola-cpcap.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/motorola-cpcap.txt
> @@ -0,0 +1,31 @@
> +Motorola CPCAP PMIC device tree binding
> +
> +Required properties:
> +- compatible : One or both of "motorola,cpcap" or "ste,6556002"
> +- reg : SPI chip select
> +- interrupt-parent : The parent interrupt controller
> +- interrupts : The interrupt line the device is connected to
> +- interrupt-controller : Marks the device node as an interrupt controller
> +- #interrupt-cells : The number of cells to describe an IRQ, should be 2
> +- #address-cells : Child device offset number of cells, should be 1
> +- #size-cells : Child device size number of cells, should be 0
> +- spi-max-frequency : Typically set to 3000000
> +- spi-cs-high : SPI chip select direction
> +
> +Example:
> +
> +&mcspi1 {
> + cpcap: pmic@0 {
> + compatible = "motorola,cpcap", "ste,6556002";
> + reg = <0>; /* cs0 */
> + interrupt-parent = <&gpio1>;
> + interrupts = <7 IRQ_TYPE_EDGE_RISING>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + spi-max-frequency = <3000000>;
> + spi-cs-high;
> + };
> +};
> +
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -714,6 +714,17 @@ config EZX_PCAP
> This enables the PCAP ASIC present on EZX Phones. This is
> needed for MMC, TouchScreen, Sound, USB, etc..
>
> +config MFD_CPCAP
> + tristate "Support for Motorola CPCAP"
> + depends on SPI
> + depends on OF || COMPILE_TEST
> + select REGMAP_SPI
> + select REGMAP_IRQ
> + help
> + Say yes here if you want to include driver for CPCAP.
> + It is used on many Motorola phones and tablets as a PMIC.
> + At least Motorola Droid 4 is known to use CPCAP.
> +
> config MFD_VIPERBOARD
> tristate "Nano River Technologies Viperboard"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> obj-$(CONFIG_MFD_CORE) += mfd-core.o
>
> obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> +obj-$(CONFIG_MFD_CPCAP) += motorola-cpcap.o
>
> obj-$(CONFIG_MCP) += mcp-core.o
> obj-$(CONFIG_MCP_SA11X0) += mcp-sa11x0.o
> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -0,0 +1,244 @@
> +/*
> + * Motorola CPCAP PMIC core driver
> + *
> + * Copyright (C) 2016 Tony Lindgren <tony@atomide.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/mfd/motorola-cpcap.h>
> +#include <linux/spi/spi.h>
> +
> +#define CPCAP_NR_IRQ_REG_BANKS 6
> +#define CPCAP_NR_IRQ_CHIPS 3
> +
> +struct cpcap_ddata {
> + struct spi_device *spi;
> + struct regmap_irq *irqs;
> + struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> + const struct regmap_config *regmap_conf;
> + struct regmap *regmap;
> +};
> +
> +static int cpcap_check_revision(struct cpcap_ddata *cpcap)
> +{
> + u16 vendor, rev;
> + int ret;
> +
> + ret = cpcap_get_vendor(&cpcap->spi->dev, cpcap->regmap, &vendor);
> + if (ret)
> + return ret;
> +
> + ret = cpcap_get_revision(&cpcap->spi->dev, cpcap->regmap, &rev);
> + if (ret)
> + return ret;
> +
> + dev_info(&cpcap->spi->dev, "CPCAP vendor: %s rev: %i.%i (%x)\n",
> + vendor == CPCAP_VENDOR_ST ? "ST" : "TI",
> + CPCAP_REVISION_MAJOR(rev), CPCAP_REVISION_MINOR(rev),
> + rev);
> +
> + if (rev < CPCAP_REVISION_2_1) {
> + dev_info(&cpcap->spi->dev,
> + "Please add old CPCAP revision support as needed\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * First two irq chips are the two private macro interrupt chips, the third
> + * irq chip is for register banks 1 - 4 and is available for drivers to use.
> + */
> +static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
> + {
> + .name = "cpcap-m2",
> + .num_regs = 1,
> + .status_base = CPCAP_REG_MI1,
> + .ack_base = CPCAP_REG_MI1,
> + .mask_base = CPCAP_REG_MIM1,
> + .use_ack = true,
> + },
> + {
> + .name = "cpcap-m2",
> + .num_regs = 1,
> + .status_base = CPCAP_REG_MI2,
> + .ack_base = CPCAP_REG_MI2,
> + .mask_base = CPCAP_REG_MIM2,
> + .use_ack = true,
> + },
> + {
> + .name = "cpcap1-4",
> + .num_regs = 4,
> + .status_base = CPCAP_REG_INT1,
> + .ack_base = CPCAP_REG_INT1,
> + .mask_base = CPCAP_REG_INTM1,
> + .type_base = CPCAP_REG_INTS1,
> + .use_ack = true,
> + },
> +};
> +
> +static int cpcap_init_irq_chip(struct cpcap_ddata *cpcap, int irq_chip,
> + int irq_start, int nr_irqs)
> +{
> + struct regmap_irq_chip *chip = &cpcap_irq_chip[irq_chip];
> + int i, ret;
> +
> + for (i = irq_start; i < irq_start + nr_irqs; i++) {
> + struct regmap_irq *cpcap_irq = &cpcap->irqs[i];
> +
> + cpcap_irq->reg_offset =
> + ((i - irq_start) / cpcap->regmap_conf->val_bits) *
> + cpcap->regmap_conf->reg_stride;
> + cpcap_irq->mask = BIT(i % cpcap->regmap_conf->val_bits);
If a code segment takes more than a few seconds to work out, it's
either too cluttered (break it down), too complex (simplify it) or if
neither of the first two approaches are possible, then a comment is
required.
> + }
> + chip->irqs = &cpcap->irqs[irq_start];
> + chip->num_irqs = nr_irqs;
> + chip->irq_drv_data = cpcap;
> +
> + ret = devm_regmap_add_irq_chip(&cpcap->spi->dev, cpcap->regmap,
> + cpcap->spi->irq,
> + IRQF_TRIGGER_RISING |
> + IRQF_SHARED, -1,
> + chip, &cpcap->irqdata[irq_chip]);
> + if (ret) {
> + dev_err(&cpcap->spi->dev, "could not add irq chip %i: %i\n",
> + irq_chip, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> +{
> + int ret;
> +
> + cpcap->irqs = devm_kzalloc(&cpcap->spi->dev,
> + sizeof(*cpcap->irqs) *
> + CPCAP_NR_IRQ_REG_BANKS *
> + cpcap->regmap_conf->val_bits,
> + GFP_KERNEL);
> + if (!cpcap->irqs)
> + return -ENOMEM;
> +
> + ret = cpcap_init_irq_chip(cpcap, 0, 0, 16);
> + if (ret)
> + return ret;
> +
> + ret = cpcap_init_irq_chip(cpcap, 1, 16, 16);
> + if (ret)
> + return ret;
> +
> + ret = cpcap_init_irq_chip(cpcap, 2, 32, 64);
> + if (ret)
> + return ret;
> +
> + enable_irq_wake(cpcap->spi->irq);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id cpcap_of_match[] = {
> + { .compatible = "motorola,cpcap", },
> + { .compatible = "st,6556002", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> +
> +static const struct regmap_config cpcap_regmap_config = {
> + .reg_bits = 16,
> + .reg_stride = 4,
> + .pad_bits = 0,
> + .val_bits = 16,
> + .write_flag_mask = 0x8000,
> + .max_register = CPCAP_REG_ST_TEST2,
> + .cache_type = REGCACHE_NONE,
> + .reg_format_endian = REGMAP_ENDIAN_LITTLE,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static int cpcap_probe(struct spi_device *spi)
> +{
> + const struct of_device_id *match;
> + int ret = -EINVAL;
What is the purpose of initialising ret?
> + struct cpcap_ddata *cpcap;
> +
> + match = of_match_device(of_match_ptr(cpcap_of_match), &spi->dev);
> + if (!match)
> + return -ENODEV;
> +
> + cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> + if (!cpcap)
> + return -ENOMEM;
> +
> + cpcap->spi = spi;
> + spi_set_drvdata(spi, cpcap);
> +
> + spi->bits_per_word = 16;
> + spi->mode = SPI_MODE_0 | SPI_CS_HIGH;
> +
> + ret = spi_setup(spi);
> + if (ret < 0)
> + return ret;
Is it possible for spi_setup() to return >0?
If not, then "if (!ret)" will do.
> + cpcap->regmap_conf = &cpcap_regmap_config;
> + cpcap->regmap = devm_regmap_init_spi(spi, &cpcap_regmap_config);
> + if (IS_ERR(cpcap->regmap)) {
> + ret = PTR_ERR(cpcap->regmap);
> + dev_err(&cpcap->spi->dev, "Failed to initialize regmap: %d\n",
> + ret);
> +
> + return ret;
> + }
> +
> + ret = cpcap_check_revision(cpcap);
> + if (ret) {
> + dev_err(&cpcap->spi->dev, "Failed to detect CPCAP: %i\n", ret);
> + return ret;
> + }
> +
> + ret = cpcap_init_irq(cpcap);
> + if (ret)
> + return ret;
> +
> + return of_platform_populate(spi->dev.of_node, NULL, NULL,
> + &cpcap->spi->dev);
> +}
> +
> +static int cpcap_remove(struct spi_device *pdev)
> +{
> + struct cpcap_ddata *cpcap = spi_get_drvdata(pdev);
> +
> + of_platform_depopulate(&cpcap->spi->dev);
> +
> + return 0;
> +}
> +
> +static struct spi_driver cpcap_driver = {
> + .driver = {
> + .name = "cpcap-core",
> + .of_match_table = cpcap_of_match,
of_match_ptr() ?
> + },
> + .probe = cpcap_probe,
> + .remove = cpcap_remove,
> +};
> +module_spi_driver(cpcap_driver);
> +
> +MODULE_ALIAS("platform:cpcap");
> +MODULE_DESCRIPTION("CPCAP driver");
> +MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h
> new file mode 100644
> --- /dev/null
> +++ b/include/linux/mfd/motorola-cpcap.h
> @@ -0,0 +1,289 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Note that the register defines are based on earlier cpcap.h in
> + * Motorola Linux kernel tree Copyright (C) 2007-2009 Motorola, Inc.
> + *
> + * Rewritten for the real register offsets instead of enumeration
> + * to make the defines usable with Linux kernel regmap support
'\n'
> + * Copyright (C) 2016 Tony Lindgren <tony@atomide.com>.
Actually, it's more traditional to place the CR at the top.
> + */
> +
> +#define CPCAP_VENDOR_ST 0
> +#define CPCAP_VENDOR_TI 1
> +
> +#define CPCAP_REVISION_MAJOR(r) (((r) >> 4) + 1)
> +#define CPCAP_REVISION_MINOR(r) ((r) & 0xf)
> +
> +#define CPCAP_REVISION_1_0 0x08
> +#define CPCAP_REVISION_1_1 0x09
> +#define CPCAP_REVISION_2_0 0x10
> +#define CPCAP_REVISION_2_1 0x11
> +
> +/* CPCAP registers */
> +#define CPCAP_REG_INT1 0x0000 /* Interrupt 1 */
> +#define CPCAP_REG_INT2 0x0004 /* Interrupt 2 */
> +#define CPCAP_REG_INT3 0x0008 /* Interrupt 3 */
> +#define CPCAP_REG_INT4 0x000c /* Interrupt 4 */
> +#define CPCAP_REG_INTM1 0x0010 /* Interrupt Mask 1 */
> +#define CPCAP_REG_INTM2 0x0014 /* Interrupt Mask 2 */
> +#define CPCAP_REG_INTM3 0x0018 /* Interrupt Mask 3 */
> +#define CPCAP_REG_INTM4 0x001c /* Interrupt Mask 4 */
> +#define CPCAP_REG_INTS1 0x0020 /* Interrupt Sense 1 */
> +#define CPCAP_REG_INTS2 0x0024 /* Interrupt Sense 2 */
> +#define CPCAP_REG_INTS3 0x0028 /* Interrupt Sense 3 */
> +#define CPCAP_REG_INTS4 0x002c /* Interrupt Sense 4 */
> +#define CPCAP_REG_ASSIGN1 0x0030 /* Resource Assignment 1 */
> +#define CPCAP_REG_ASSIGN2 0x0034 /* Resource Assignment 2 */
> +#define CPCAP_REG_ASSIGN3 0x0038 /* Resource Assignment 3 */
> +#define CPCAP_REG_ASSIGN4 0x003c /* Resource Assignment 4 */
> +#define CPCAP_REG_ASSIGN5 0x0040 /* Resource Assignment 5 */
> +#define CPCAP_REG_ASSIGN6 0x0044 /* Resource Assignment 6 */
> +#define CPCAP_REG_VERSC1 0x0048 /* Version Control 1 */
> +#define CPCAP_REG_VERSC2 0x004c /* Version Control 2 */
> +
> +#define CPCAP_REG_MI1 0x0200 /* Macro Interrupt 1 */
> +#define CPCAP_REG_MIM1 0x0204 /* Macro Interrupt Mask 1 */
> +#define CPCAP_REG_MI2 0x0208 /* Macro Interrupt 2 */
> +#define CPCAP_REG_MIM2 0x020c /* Macro Interrupt Mask 2 */
> +#define CPCAP_REG_UCC1 0x0210 /* UC Control 1 */
> +#define CPCAP_REG_UCC2 0x0214 /* UC Control 2 */
> +
> +#define CPCAP_REG_PC1 0x021c /* Power Cut 1 */
> +#define CPCAP_REG_PC2 0x0220 /* Power Cut 2 */
> +#define CPCAP_REG_BPEOL 0x0224 /* BP and EOL */
> +#define CPCAP_REG_PGC 0x0228 /* Power Gate and Control */
> +#define CPCAP_REG_MT1 0x022c /* Memory Transfer 1 */
> +#define CPCAP_REG_MT2 0x0230 /* Memory Transfer 2 */
> +#define CPCAP_REG_MT3 0x0234 /* Memory Transfer 3 */
> +#define CPCAP_REG_PF 0x0238 /* Print Format */
> +
> +#define CPCAP_REG_SCC 0x0400 /* System Clock Control */
> +#define CPCAP_REG_SW1 0x0404 /* Stop Watch 1 */
> +#define CPCAP_REG_SW2 0x0408 /* Stop Watch 2 */
> +#define CPCAP_REG_UCTM 0x040c /* UC Turbo Mode */
> +#define CPCAP_REG_TOD1 0x0410 /* Time of Day 1 */
> +#define CPCAP_REG_TOD2 0x0414 /* Time of Day 2 */
> +#define CPCAP_REG_TODA1 0x0418 /* Time of Day Alarm 1 */
> +#define CPCAP_REG_TODA2 0x041c /* Time of Day Alarm 2 */
> +#define CPCAP_REG_DAY 0x0420 /* Day */
> +#define CPCAP_REG_DAYA 0x0424 /* Day Alarm */
> +#define CPCAP_REG_VAL1 0x0428 /* Validity 1 */
> +#define CPCAP_REG_VAL2 0x042c /* Validity 2 */
> +
> +#define CPCAP_REG_SDVSPLL 0x0600 /* Switcher DVS and PLL */
> +#define CPCAP_REG_SI2CC1 0x0604 /* Switcher I2C Control 1 */
> +#define CPCAP_REG_Si2CC2 0x0608 /* Switcher I2C Control 2 */
> +#define CPCAP_REG_S1C1 0x060c /* Switcher 1 Control 1 */
> +#define CPCAP_REG_S1C2 0x0610 /* Switcher 1 Control 2 */
> +#define CPCAP_REG_S2C1 0x0614 /* Switcher 2 Control 1 */
> +#define CPCAP_REG_S2C2 0x0618 /* Switcher 2 Control 2 */
> +#define CPCAP_REG_S3C 0x061c /* Switcher 3 Control */
> +#define CPCAP_REG_S4C1 0x0620 /* Switcher 4 Control 1 */
> +#define CPCAP_REG_S4C2 0x0624 /* Switcher 4 Control 2 */
> +#define CPCAP_REG_S5C 0x0628 /* Switcher 5 Control */
> +#define CPCAP_REG_S6C 0x062c /* Switcher 6 Control */
> +#define CPCAP_REG_VCAMC 0x0630 /* VCAM Control */
> +#define CPCAP_REG_VCSIC 0x0634 /* VCSI Control */
> +#define CPCAP_REG_VDACC 0x0638 /* VDAC Control */
> +#define CPCAP_REG_VDIGC 0x063c /* VDIG Control */
> +#define CPCAP_REG_VFUSEC 0x0640 /* VFUSE Control */
> +#define CPCAP_REG_VHVIOC 0x0644 /* VHVIO Control */
> +#define CPCAP_REG_VSDIOC 0x0648 /* VSDIO Control */
> +#define CPCAP_REG_VPLLC 0x064c /* VPLL Control */
> +#define CPCAP_REG_VRF1C 0x0650 /* VRF1 Control */
> +#define CPCAP_REG_VRF2C 0x0654 /* VRF2 Control */
> +#define CPCAP_REG_VRFREFC 0x0658 /* VRFREF Control */
> +#define CPCAP_REG_VWLAN1C 0x065c /* VWLAN1 Control */
> +#define CPCAP_REG_VWLAN2C 0x0660 /* VWLAN2 Control */
> +#define CPCAP_REG_VSIMC 0x0664 /* VSIM Control */
> +#define CPCAP_REG_VVIBC 0x0668 /* VVIB Control */
> +#define CPCAP_REG_VUSBC 0x066c /* VUSB Control */
> +#define CPCAP_REG_VUSBINT1C 0x0670 /* VUSBINT1 Control */
> +#define CPCAP_REG_VUSBINT2C 0x0674 /* VUSBINT2 Control */
> +#define CPCAP_REG_URT 0x0678 /* Useroff Regulator Trigger */
> +#define CPCAP_REG_URM1 0x067c /* Useroff Regulator Mask 1 */
> +#define CPCAP_REG_URM2 0x0680 /* Useroff Regulator Mask 2 */
> +
> +#define CPCAP_REG_VAUDIOC 0x0800 /* VAUDIO Control */
> +#define CPCAP_REG_CC 0x0804 /* Codec Control */
> +#define CPCAP_REG_CDI 0x0808 /* Codec Digital Interface */
> +#define CPCAP_REG_SDAC 0x080c /* Stereo DAC */
> +#define CPCAP_REG_SDACDI 0x0810 /* Stereo DAC Digital Interface */
> +#define CPCAP_REG_TXI 0x0814 /* TX Inputs */
> +#define CPCAP_REG_TXMP 0x0818 /* TX MIC PGA's */
> +#define CPCAP_REG_RXOA 0x081c /* RX Output Amplifiers */
> +#define CPCAP_REG_RXVC 0x0820 /* RX Volume Control */
> +#define CPCAP_REG_RXCOA 0x0824 /* RX Codec to Output Amps */
> +#define CPCAP_REG_RXSDOA 0x0828 /* RX Stereo DAC to Output Amps */
> +#define CPCAP_REG_RXEPOA 0x082c /* RX External PGA to Output Amps */
> +#define CPCAP_REG_RXLL 0x0830 /* RX Low Latency */
> +#define CPCAP_REG_A2LA 0x0834 /* A2 Loudspeaker Amplifier */
> +#define CPCAP_REG_MIPIS1 0x0838 /* MIPI Slimbus 1 */
> +#define CPCAP_REG_MIPIS2 0x083c /* MIPI Slimbus 2 */
> +#define CPCAP_REG_MIPIS3 0x0840 /* MIPI Slimbus 3. */
> +#define CPCAP_REG_LVAB 0x0844 /* LMR Volume and A4 Balanced. */
> +
> +#define CPCAP_REG_CCC1 0x0a00 /* Coulomb Counter Control 1 */
> +#define CPCAP_REG_CRM 0x0a04 /* Charger and Reverse Mode */
> +#define CPCAP_REG_CCCC2 0x0a08 /* Coincell and Coulomb Ctr Ctrl 2 */
> +#define CPCAP_REG_CCS1 0x0a0c /* Coulomb Counter Sample 1 */
> +#define CPCAP_REG_CCS2 0x0a10 /* Coulomb Counter Sample 2 */
> +#define CPCAP_REG_CCA1 0x0a14 /* Coulomb Counter Accumulator 1 */
> +#define CPCAP_REG_CCA2 0x0a18 /* Coulomb Counter Accumulator 2 */
> +#define CPCAP_REG_CCM 0x0a1c /* Coulomb Counter Mode */
> +#define CPCAP_REG_CCO 0x0a20 /* Coulomb Counter Offset */
> +#define CPCAP_REG_CCI 0x0a24 /* Coulomb Counter Integrator */
> +
> +#define CPCAP_REG_ADCC1 0x0c00 /* A/D Converter Configuration 1 */
> +#define CPCAP_REG_ADCC2 0x0c04 /* A/D Converter Configuration 2 */
> +#define CPCAP_REG_ADCD0 0x0c08 /* A/D Converter Data 0 */
> +#define CPCAP_REG_ADCD1 0x0c0c /* A/D Converter Data 1 */
> +#define CPCAP_REG_ADCD2 0x0c10 /* A/D Converter Data 2 */
> +#define CPCAP_REG_ADCD3 0x0c14 /* A/D Converter Data 3 */
> +#define CPCAP_REG_ADCD4 0x0c18 /* A/D Converter Data 4 */
> +#define CPCAP_REG_ADCD5 0x0c1c /* A/D Converter Data 5 */
> +#define CPCAP_REG_ADCD6 0x0c20 /* A/D Converter Data 6 */
> +#define CPCAP_REG_ADCD7 0x0c24 /* A/D Converter Data 7 */
> +#define CPCAP_REG_ADCAL1 0x0c28 /* A/D Converter Calibration 1 */
> +#define CPCAP_REG_ADCAL2 0x0c2c /* A/D Converter Calibration 2 */
> +
> +#define CPCAP_REG_USBC1 0x0e00 /* USB Control 1 */
> +#define CPCAP_REG_USBC2 0x0e04 /* USB Control 2 */
> +#define CPCAP_REG_USBC3 0x0e08 /* USB Control 3 */
> +#define CPCAP_REG_UVIDL 0x0e0c /* ULPI Vendor ID Low */
> +#define CPCAP_REG_UVIDH 0x0e10 /* ULPI Vendor ID High */
> +#define CPCAP_REG_UPIDL 0x0e14 /* ULPI Product ID Low */
> +#define CPCAP_REG_UPIDH 0x0e18 /* ULPI Product ID High */
> +#define CPCAP_REG_UFC1 0x0e1c /* ULPI Function Control 1 */
> +#define CPCAP_REG_UFC2 0x0e20 /* ULPI Function Control 2 */
> +#define CPCAP_REG_UFC3 0x0e24 /* ULPI Function Control 3 */
> +#define CPCAP_REG_UIC1 0x0e28 /* ULPI Interface Control 1 */
> +#define CPCAP_REG_UIC2 0x0e2c /* ULPI Interface Control 2 */
> +#define CPCAP_REG_UIC3 0x0e30 /* ULPI Interface Control 3 */
> +#define CPCAP_REG_USBOTG1 0x0e34 /* USB OTG Control 1 */
> +#define CPCAP_REG_USBOTG2 0x0e38 /* USB OTG Control 2 */
> +#define CPCAP_REG_USBOTG3 0x0e3c /* USB OTG Control 3 */
> +#define CPCAP_REG_UIER1 0x0e40 /* USB Interrupt Enable Rising 1 */
> +#define CPCAP_REG_UIER2 0x0e44 /* USB Interrupt Enable Rising 2 */
> +#define CPCAP_REG_UIER3 0x0e48 /* USB Interrupt Enable Rising 3 */
> +#define CPCAP_REG_UIEF1 0x0e4c /* USB Interrupt Enable Falling 1 */
> +#define CPCAP_REG_UIEF2 0x0e50 /* USB Interrupt Enable Falling 1 */
> +#define CPCAP_REG_UIEF3 0x0e54 /* USB Interrupt Enable Falling 1 */
> +#define CPCAP_REG_UIS 0x0e58 /* USB Interrupt Status */
> +#define CPCAP_REG_UIL 0x0e5c /* USB Interrupt Latch */
> +#define CPCAP_REG_USBD 0x0e60 /* USB Debug */
> +#define CPCAP_REG_SCR1 0x0e64 /* Scratch 1 */
> +#define CPCAP_REG_SCR2 0x0e68 /* Scratch 2 */
> +#define CPCAP_REG_SCR3 0x0e6c /* Scratch 3 */
> +
> +#define CPCAP_REG_VMC 0x0eac /* Video Mux Control */
> +#define CPCAP_REG_OWDC 0x0eb0 /* One Wire Device Control */
> +#define CPCAP_REG_GPIO0 0x0eb4 /* GPIO 0 Control */
> +
> +#define CPCAP_REG_GPIO1 0x0ebc /* GPIO 1 Control */
> +
> +#define CPCAP_REG_GPIO2 0x0ec4 /* GPIO 2 Control */
> +
> +#define CPCAP_REG_GPIO3 0x0ecc /* GPIO 3 Control */
> +
> +#define CPCAP_REG_GPIO4 0x0ed4 /* GPIO 4 Control */
> +
> +#define CPCAP_REG_GPIO5 0x0edc /* GPIO 5 Control */
> +
> +#define CPCAP_REG_GPIO6 0x0ee4 /* GPIO 6 Control */
> +
> +#define CPCAP_REG_MDLC 0x1000 /* Main Display Lighting Control */
> +#define CPCAP_REG_KLC 0x1004 /* Keypad Lighting Control */
> +#define CPCAP_REG_ADLC 0x1008 /* Aux Display Lighting Control */
> +#define CPCAP_REG_REDC 0x100c /* Red Triode Control */
> +#define CPCAP_REG_GREENC 0x1010 /* Green Triode Control */
> +#define CPCAP_REG_BLUEC 0x1014 /* Blue Triode Control */
> +#define CPCAP_REG_CFC 0x1018 /* Camera Flash Control */
> +#define CPCAP_REG_ABC 0x101c /* Adaptive Boost Control */
> +#define CPCAP_REG_BLEDC 0x1020 /* Bluetooth LED Control */
> +#define CPCAP_REG_CLEDC 0x1024 /* Camera Privacy LED Control */
> +
> +#define CPCAP_REG_OW1C 0x1200 /* One Wire 1 Command */
> +#define CPCAP_REG_OW1D 0x1204 /* One Wire 1 Data */
> +#define CPCAP_REG_OW1I 0x1208 /* One Wire 1 Interrupt */
> +#define CPCAP_REG_OW1IE 0x120c /* One Wire 1 Interrupt Enable */
> +
> +#define CPCAP_REG_OW1 0x1214 /* One Wire 1 Control */
> +
> +#define CPCAP_REG_OW2C 0x1220 /* One Wire 2 Command */
> +#define CPCAP_REG_OW2D 0x1224 /* One Wire 2 Data */
> +#define CPCAP_REG_OW2I 0x1228 /* One Wire 2 Interrupt */
> +#define CPCAP_REG_OW2IE 0x122c /* One Wire 2 Interrupt Enable */
> +
> +#define CPCAP_REG_OW2 0x1234 /* One Wire 2 Control */
> +
> +#define CPCAP_REG_OW3C 0x1240 /* One Wire 3 Command */
> +#define CPCAP_REG_OW3D 0x1244 /* One Wire 3 Data */
> +#define CPCAP_REG_OW3I 0x1248 /* One Wire 3 Interrupt */
> +#define CPCAP_REG_OW3IE 0x124c /* One Wire 3 Interrupt Enable */
> +
> +#define CPCAP_REG_OW3 0x1254 /* One Wire 3 Control */
> +#define CPCAP_REG_GCAIC 0x1258 /* GCAI Clock Control */
> +#define CPCAP_REG_GCAIM 0x125c /* GCAI GPIO Mode */
> +#define CPCAP_REG_LGDIR 0x1260 /* LMR GCAI GPIO Direction */
> +#define CPCAP_REG_LGPU 0x1264 /* LMR GCAI GPIO Pull-up */
> +#define CPCAP_REG_LGPIN 0x1268 /* LMR GCAI GPIO Pin */
> +#define CPCAP_REG_LGMASK 0x126c /* LMR GCAI GPIO Mask */
> +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> +
> +#define CPCAP_REG_TEST 0x7c00 /* Test */
> +
> +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> +
> +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
> +
> +/*
> + * Helpers for child devices to check the revision and vendor.
> + *
> + * REVISIT: No documentation for the bits below, please update
> + * to use proper names for defines when available.
> + */
> +
> +static inline int cpcap_get_revision(struct device *dev,
> + struct regmap *regmap,
> + u16 *revision)
> +{
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(regmap, CPCAP_REG_VERSC1, &val);
> + if (ret) {
> + dev_err(dev, "Could not read revision\n");
> +
> + return ret;
> + }
> +
> + *revision = ((val >> 3) & 0x7) | ((val << 3) & 0x38);
> +
> + return 0;
> +}
> +
> +static inline int cpcap_get_vendor(struct device *dev,
> + struct regmap *regmap,
> + u16 *vendor)
> +{
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(regmap, CPCAP_REG_VERSC1, &val);
> + if (ret) {
> + dev_err(dev, "Could not read vendor\n");
> +
> + return ret;
> + }
> +
> + *vendor = (val >> 6) & 0x7;
> +
> + return 0;
> +}
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2017-01-03 16:33 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <CAO7Z3WJa0goJ-VXc7dvyz8imZtqby6QsC0QNH+uRAE8LhxqU2w@mail.gmail.com>
[Please stop top-posting. Bottom-post only to these lists.]
Hi Geoff & happy new year.
On Tue, Dec 27, 2016 at 09:18:32AM -0500, Geoff Lansberry wrote:
> Mark - I will split this off soon.
OK
> In the meantime - here is some more info about how we use it.
>
> We do use NFC structures. I did find an interesting clue in that
> there are certain bottles that cause neard to segfault, I'm not sure
> what is different about them. We write a string, like
> "coppola_chardonnay_2015" to the bottles.
Off the top of my head, it could be the length of the text.
It would be useful to compare the data that works to the data
that doesn't work. Can you install NXP's 'TagInfo' app on a
smartphone and scan tags with working & non-working data?
You can email the data from the app to yourself, edit out
the cruft, and share here.
> Come to think of it, I
> haven't done anything special to make that an ndef record, just
> assumed that it would happen by default, I'll look into this further.
If you wrote the data using neard, it will be NDEF formatted.
Since it is working this well, it is virtually guaranteed that
the data is NDEF formatted.
> Also, I've been running neard with --plugin nfctype2. Just in case
> the problem was happening due to cycling through other tag types. It
> didn't seem to make any difference, but I have not gone back to
> default.
Good to know, thanks.
Mark
--
^ permalink raw reply
* Re: [PATCH 0/8] ARM: dts: Switch to new DSA binding
From: Andrew Lunn @ 2017-01-03 16:32 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Florian Fainelli, Jason Cooper, vivien.didelot, Russell King,
open list, Rob Herring, linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <87eg0kf75m.fsf@free-electrons.com>
> The series looks OK. However I would like to have a reviewed by from
> Andrew who know well the mvebu platform and the DSA subsystem.
Hi Gregory
Yes, i was planning on reviewing, and testing on at least three of
these platforms.
Since this is mvebu/arm-soc and not netdev, i may take a week or so
before i get around to it. netdev is too fast some times.
Andrew
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: exynos: make tm2 and tm2e independent from each other
From: Krzysztof Kozlowski @ 2017-01-03 16:31 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Mark Rutland, devicetree, linux-samsung-soc, Kukjin Kim,
Chanwoo Choi, Catalin Marinas, Jaechul Lee, Dmitry Torokhov,
Will Deacon, linux-kernel, Andi Shyti, Javier Martinez Canillas,
Rob Herring, Krzysztof Kozlowski, Andi Shyti, linux-input,
galaxyra, beomho.seo, linux-arm-kernel
In-Reply-To: <CAGTfZH0tnjkqF_7DChLSvzs3C369mkb_HZmyrf3cc0T-UcBZ4A@mail.gmail.com>
On Wed, Jan 04, 2017 at 12:29:23AM +0900, Chanwoo Choi wrote:
> Hi Andi,
>
> 2017-01-03 23:40 GMT+09:00 Andi Shyti <andi@etezian.org>:
> > Hi,
> >
> >> FWIW, I also agree with Chanwoo that the difference is too small to
> >> need a common .dtsi file.
> >
> > in principle I don't like "switching on and off" properties by
> > overwriting them with "status = disable", unless it's really
> > necessary (and this case is not). Even for small differences. It
> > makes the DTS harder to read and duplicates nodes with different
> > values throughout the DTS include chain.
I agree.
> >
> > In my opinion this approach should be discouraged.
> >
> > Besides, there are other overwritten differences in tm2e.dts that
> > I think should be separated as well. The "common" file approach is
> > widely used in arm/boot/dts/exynos* files.
I agree. Mostly we use:
1. Common DTSI and final addons/customizations in DTS.
2. Several common DTSI for specific parts (like sound).
> > The "status = disable" looks to me more like a temporary hack
> > rather than a permanent solution.
> >
> > In any case, still up to you :)
> >
> > Andi
>
> I think that "status=disabled" of hsi2c_9 is not hack. The overwrite
> is possible for Device-tree. But, there is just difference how to
> support them with some method.
>
> Except for touchkey, all peripheral device are same on both tm2 and
> tm2e. There are only small difference for a few property value.
>
> To understand the difference between tm2 and tm2e, I made the patch
> (it is not complete version). If we implement the following patch, we
> support both tm2 and tm2e. So, I think that it is not complex to
> understand the h/w difference between tm2 and tm2e.
The difference below (I removed it from the quote) is indeed very small,
thanks Chanwoo for pointing this. However having common DTSI should not
end with much bigger final diff between files. I mean that it should be
the same amount of lines in total...
Actually, after applying this patch, the final exynos5433-tm2e.dts is
exactly the same as before. To me, this is a indication that a common
file is a good approach.
>From a separate Javier's mail:
> On 01/03/2017 11:40 AM, Andi Shyti wrote:
> > Hi,
> >
> >> FWIW, I also agree with Chanwoo that the difference is too small to
> >> need a common .dtsi file.
> >
> > in principle I don't like "switching on and off" properties by
> > overwriting them with "status = disable", unless it's really
>
> This is a very good point. It would had been different if it was the
> opposite and tm2e had to enable the device node, but disabling it is
> indeed more confusing.
> On 01/03/2017 11:40 AM, Andi Shyti wrote:
> > Hi,
> >
> >> FWIW, I also agree with Chanwoo that the difference is too small to
> >> need a common .dtsi file.
> >
> > in principle I don't like "switching on and off" properties by
> > overwriting them with "status = disable", unless it's really
>
> This is a very good point. It would had been different if it was the
> opposite and tm2e had to enable the device node, but disabling it is
> indeed more confusing.
I agree, 'status = disable' in final DTS is not a common pattern and
to me when reading, it makes me wonder what was the author's intention.
Also including a DTS in DTS is not a common pattern, neither. It appears
(git grep include arch/arm/boot/dts | grep 'dts"') in few places but it
is not obvious. At least to me.
Overall, I prefer the common approach.
Best regards,
Krzysztof
^ 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