* 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 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 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 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 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 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 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 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 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 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 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 1/6] mfd: dt: Fix "indicates" typo in mfd bindings document
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-2-andrew@aj.id.au>
On Tue, 06 Dec 2016, Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Applied with Acks, thanks.
> ---
> Documentation/devicetree/bindings/mfd/mfd.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> index af9d6931a1a2..f1fceeda12f1 100644
> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -19,7 +19,7 @@ Optional properties:
>
> - compatible : "simple-mfd" - this signifies that the operating system should
> consider all subnodes of the MFD device as separate devices akin to how
> - "simple-bus" inidicates when to see subnodes as children for a simple
> + "simple-bus" indicates when to see subnodes as children for a simple
> memory-mapped bus. For more complex devices, when the nexus driver has to
> probe registers to figure out what child devices exist etc, this should not
> be used. In the latter case the child devices will be determined by the
--
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 v3 5/6] mfd: dt: Add bindings for the Aspeed SoC Display Controller (GFX)
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-6-andrew@aj.id.au>
On Tue, 06 Dec 2016, Andrew Jeffery wrote:
> The Aspeed SoC Display Controller is presented as a syscon device to
> arbitrate access by display and pinmux drivers. Video pinmux
> configuration on fifth generation SoCs depends on bits in both the
> System Control Unit and the Display Controller.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Acked-by: Rob Herring <robh@kernel.org>
Applied with Acks, thanks.
> ---
> .../devicetree/bindings/mfd/syscon/aspeed-gfx.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/syscon/aspeed-gfx.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/syscon/aspeed-gfx.txt b/Documentation/devicetree/bindings/mfd/syscon/aspeed-gfx.txt
> new file mode 100644
> index 000000000000..aea5370efd97
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/syscon/aspeed-gfx.txt
> @@ -0,0 +1,17 @@
> +* Device tree bindings for Aspeed SoC Display Controller (GFX)
> +
> +The Aspeed SoC Display Controller primarily does as its name suggests, but also
> +participates in pinmux requests on the g5 SoCs. It is therefore considered a
> +syscon device.
> +
> +Required properties:
> +- compatible: "aspeed,ast2500-gfx", "syscon"
> +- reg: contains offset/length value of the GFX memory
> + region.
> +
> +Example:
> +
> +gfx: display@1e6e6000 {
> + compatible = "aspeed,ast2500-gfx", "syscon";
> + reg = <0x1e6e6000 0x1000>;
> +};
--
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 v7 1/2] Add OV5647 device tree documentation
From: Rob Herring @ 2017-01-03 18:02 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <2df6779f915edc7eb48ef215235d480f62f165b0.1482846784.git.roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On Tue, Dec 27, 2016 at 01:59:02PM +0000, Ramiro Oliveira wrote:
> Create device tree bindings documentation.
>
> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> ---
> .../devicetree/bindings/media/i2c/ov5647.txt | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/4] Documetation: samsung-phy: add the exynos-pcie-phy binding
From: Rob Herring @ 2017-01-03 18:05 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
mark.rutland, kgene, krzk, kishon, jingoohan1, vivek.gautam,
pankaj.dubey, alim.akhtar, cpgs
In-Reply-To: <20161228103454.26467-3-jh80.chung@samsung.com>
On Wed, Dec 28, 2016 at 07:34:52PM +0900, Jaehoon Chung wrote:
> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
> This is for using generic PHY framework.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> .../devicetree/bindings/phy/samsung-phy.txt | 23 ++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index 9872ba8..1cbc15f 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -191,3 +191,26 @@ Example:
> usbdrdphy0 = &usb3_phy0;
> usbdrdphy1 = &usb3_phy1;
> };
> +
> +Samsung Exynos SoC series PCIe PHY controller
> +--------------------------------------------------
> +Required properties:
> +- compatible : Should be set to "samsung,exynos5440-pcie-phy"
> +- #phy-cells : Must be zero
> +- reg : a register used by phy driver.
> +
> +Required properies for child node:
> +- reg : a block register used by phy driver.
There's no need for this. Either just make the length 0x1040 or add a
2nd address to the parent reg prop.
> +
> +Example:
> + pcie_phy0: pcie-phy@270000 {
> + #phy-cells = <0>;
> + compatible = "samsung,exynos5440-pcie-phy";
> + reg = <0x270000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + block@271000 {
> + reg = <0x271000 0x40>;
> + };
> + };
> --
> 2.10.2
>
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Geoff Lansberry @ 2017-01-03 18:35 UTC (permalink / raw)
To: Mark Greer
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: <20170103163324.GA3184@animalcreek.com>
On Tue, Jan 3, 2017 at 11:33 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> [Please stop top-posting. Bottom-post only to these lists.]
Sorry; gmail keeps baiting me to do it...
>
> 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
Thanks for the reminder!
>
>> 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.
The data is always the same - and the tags are all the same. Only
difference is that the tag is physically different, and perhaps
orientation; distance from antenna to tag is fixed. I can't even
write the tags at all, so reading them will show blank. Also a minor
but significant detail, is that the tags are embedded in such a way
that the phone cannot get close enough to them to connect.
>
>> 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.
OK, good.
>
>> 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 v3 1/3] power: reset: add linkstation-reset driver
From: Florian Fainelli @ 2017-01-03 18:39 UTC (permalink / raw)
To: Roger Shimizu, Andrew Lunn, Sebastian Reichel
Cc: Martin Michlmayr, Sylver Bruneau, Herbert Valerio Riedel,
Ryan Tandy, linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAEQ9gE=MoQcr3eX0DAxZtvx0FW9pzgkUGjdxKHcsKwH7_+UsUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 01/03/2017 06:08 AM, Roger Shimizu wrote:
> Dear Florian, Andrew,
>
> Thanks for your email!
>
> On Tue, Jan 3, 2017 at 2:19 PM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> Interestingly, I submitted a patch doing nearly the same thing recently
>> after hacking on a Buffalo Terastation Pro II two days after yours
>> without seeing yours:
>>
>> https://lkml.org/lkml/2016/12/28/273
>
> Glad to know there's other developer working on linkstation/kurobox platform!
>
>> Some comments below.
>>
>>> +
>>> +static void __iomem *base;
>>> +static unsigned long tclk;
>>> +static const struct reset_cfg *cfg;
>>
>> How about avoiding the singletons here and pass this down from the
>> platform driver's private data after we (see below) also make use of a
>> reboot notifier?
>
> I see your patches. Indeed, it's a good idea to avoid the singletons
> and use private data instead.
>
>>> +static int linkstation_reset_probe(struct platform_device *pdev)
>>> +{
>>> + struct device_node *np = pdev->dev.of_node;
>>> + struct resource *res;
>>> + struct clk *clk;
>>> + char symname[KSYM_NAME_LEN];
>>> +
>>> + const struct of_device_id *match =
>>> + of_match_node(linkstation_reset_of_match_table, np);
>>> + cfg = match->data;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + if (!res) {
>>> + dev_err(&pdev->dev, "Missing resource");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>>> + if (!base) {
>>> + dev_err(&pdev->dev, "Unable to map resource");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* We need to know tclk in order to calculate the UART divisor */
>>> + clk = devm_clk_get(&pdev->dev, NULL);
>>> + if (IS_ERR(clk)) {
>>> + dev_err(&pdev->dev, "Clk missing");
>>> + return PTR_ERR(clk);
>>> + }
>>> +
>>> + tclk = clk_get_rate(clk);
>>
>> Does this work with the Terastation II which has not been converted to
>> DT yet? Is tclk available through the CLK API there?
>
> I have no idea whether device-based legacy code and make use of power
> reset driver.
> Maybe Sebastian Reichel can offer a comment?
>
> However, I think Terastation II should convert to DT first.
> If you're willing to test, I can help to provide a dts/dtb.
> (If you use Debian, I can even provide DEB of kernel image and
> flash-kernel patch, which is easy for you to test)
>
> On Tue, Jan 3, 2017 at 10:09 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>>>> +
>>>> + /* Check that nothing else has already setup a handler */
>>>> + if (pm_power_off) {
>>>> + lookup_symbol_name((ulong)pm_power_off, symname);
>>>> + dev_err(&pdev->dev,
>>>> + "pm_power_off already claimed %p %s",
>>>> + pm_power_off, symname);
>>>> + return -EBUSY;
>>>> + }
>>>> + pm_power_off = linkstation_reset;
>>>
>>> That seems a bit complicated, why not just assume that there will be
>>> either this driver used, or not at all?
>>
>> That is probably my fault. This is a copy from code i wrote many years
>> ago for the QNAP. I guess at the time i was battling with two
>> different pm_power_off handlers, so put in this code.
>>
>>> Also, you are supposed to register a reboot notifier to which you can
>>> pass private context:
>>
>> At the time i wrote the QNAP code, this did not exist. So maybe my
>> code is no longer a good example to copy.
>
> Really appreciated, Andrew!
> I'll modify this part in next series.
>
> BTW. the private data passing to reboot notifier can be shared to
> power-off function as well?
> Do you have example?
>
>> https://lkml.org/lkml/2016/12/28/275
>>
>> As indicated in my patch series, the UART1-attached micro controller
>> does a lot more things that just providing reboot, which is why I chose
>> to move this code to a MFD driver, as the starting point before adding
>> support for LEDs, FAN, PWM, beeper which would be other types of devices.
>>
>> Is adding support for other peripherals on your TODO as well?
>
> Except reset feature (power-off and reboot), other feature, such as
> LEDs / FAN speed / buttons, is managed by user-land program micro-evtd
> [0][1].
> Since the upstream [1] is not active anymore, Ryan Tandy (in CC) and I
> are maintaining it in Debian [0].
>
> [0] https://tracker.debian.org/pkg/micro-evtd
> [1] https://sourceforge.net/projects/ppc-evtd
>
> micro-evtd worked well for device-based legacy code, but after DT
> conversion, Ryan found the device cannot shutdown properly (reboot is
> OK).
> That why I created this patch series.
> I think for such old hardware and mature user-land program, it doesn't
> deserve the effort to implement those again in kernel side.
> What do you think?
An argument could be made that these are hardware peripherals, and so
the kernel is the best place to abstract support for that, and present
you with the standard device drive model for such kind of peripherals.
We don't have to do everything at the same time, so first things first,
get the reboot working, have me convert the Terastation over to Device
Tree and then we can decide what to do with these additional peripherals.
Thanks!
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/4] input: tm2-touchkey: Add touchkey driver support for TM2
From: Dmitry Torokhov @ 2017-01-03 18:56 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Jaechul Lee, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, Kukjin Kim, Krzysztof Kozlowski, Andi Shyti,
Chanwoo Choi, beomho.seo, galaxyra, linux-arm-kernel, linux-input,
devicetree, linux-kernel, linux-samsung-soc
In-Reply-To: <7794c9b2-48e6-1451-9a3e-fe24410a145d@osg.samsung.com>
On Tue, Jan 03, 2017 at 10:11:39AM -0300, Javier Martinez Canillas wrote:
> Hello Jaechul,
>
> On 01/03/2017 04:57 AM, Jaechul Lee wrote:
> > This patch adds support for the TM2 touch key and led
> > functionlity.
> >
>
> s/functionlity/functionality
>
> > 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
There should be LED dependency as well.
> > + help
> > + Say Y here to enable the tm2-touchkey.
> > + touchkey driver for tm2. This driver can enable
> > + the interrupt and make input events and control led brightness.
> > +
> > + To compile this driver as a module, choose M here.
> > + module will be called tm2-touchkey
> > +
> > config KEYBOARD_TWL4030
> > tristate "TI TWL4030/TWL5030/TPS659x0 keypad support"
> > depends on TWL4030_CORE
> > diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> > index d9f4cfc..7d9acff 100644
> > --- a/drivers/input/keyboard/Makefile
> > +++ b/drivers/input/keyboard/Makefile
> > @@ -61,6 +61,7 @@ obj-$(CONFIG_KEYBOARD_SUN4I_LRADC) += sun4i-lradc-keys.o
> > obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> > obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> > obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> > +obj-$(CONFIG_KEYBOARD_TM2_TOUCHKEY) += tm2-touchkey.o
> > obj-$(CONFIG_KEYBOARD_TWL4030) += twl4030_keypad.o
> > obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o
> > obj-$(CONFIG_KEYBOARD_W90P910) += w90p910_keypad.o
> > diff --git a/drivers/input/keyboard/tm2-touchkey.c b/drivers/input/keyboard/tm2-touchkey.c
> > new file mode 100644
> > index 0000000..d9575d8
> > --- /dev/null
> > +++ b/drivers/input/keyboard/tm2-touchkey.c
> > @@ -0,0 +1,326 @@
> > +/*
> > + * Driver for keys on GPIO lines capable of generating interrupts.
This does not match what the driver does.
> > + *
> > + * Copyright 2005 Phil Blundell
> > + * Copyright 2016 Samsung Electronics Co., Ltd.
> > + *
> > + * Author: Beomho Seo <beomho.seo@samsung.com>
> > + * Author: Jaechul Lee <jcsing.lee@samsung.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/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/leds.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/pm.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/workqueue.h>
> > +
> > +#define TM2_TOUCHKEY_DEV_NAME "tm2-touchkey"
> > +#define TM2_TOUCHKEY_KEYCODE_REG 0x03
> > +#define TM2_TOUCHKEY_BASE_REG 0x00
> > +#define TM2_TOUCHKEY_CMD_LED_ON 0x10
> > +#define TM2_TOUCHKEY_CMD_LED_OFF 0x20
> > +#define TM2_TOUCHKEY_BIT_PRESS_EV BIT(3)
> > +#define TM2_TOUCHKEY_BIT_KEYCODE GENMASK(2, 0)
> > +#define TM2_TOUCHKEY_LED_VOLTAGE_MIN 2500000
> > +#define TM2_TOUCHKEY_LED_VOLTAGE_MAX 3300000
> > +
> > +enum {
> > + TM2_TOUCHKEY_KEY_MENU = 0x1,
> > + TM2_TOUCHKEY_KEY_BACK,
> > +};
> > +
> > +#define tm2_touchkey_power_enable(x) __tm2_touchkey_power_onoff(x, 1)
> > +#define tm2_touchkey_power_disable(x) __tm2_touchkey_power_onoff(x, 0)
> > +
> > +struct tm2_touchkey_data {
> > + struct i2c_client *client;
> > + struct input_dev *input_dev;
> > + struct led_classdev led_dev;
> > +
> > + u8 keycode_type;
> > + u8 pressed;
> > + struct work_struct irq_work;
> > +
> > + bool power_onoff;
> > + struct regulator *regulator_vcc; /* 1.8V */
> > + struct regulator *regulator_vdd; /* 3.3V */
> > +};
> > +
> > +static void tm2_touchkey_led_brightness_set(struct led_classdev *led_dev,
> > + enum led_brightness brightness)
> > +{
> > + struct tm2_touchkey_data *samsung_touchkey =
Just call it "touchkey", "samsung_touchkey" is too long for a local
variable.
> > + container_of(led_dev, struct tm2_touchkey_data, led_dev);
> > + u32 volt;
> > + u8 data;
> > +
> > + if (brightness == LED_OFF) {
> > + volt = TM2_TOUCHKEY_LED_VOLTAGE_MIN;
> > + data = TM2_TOUCHKEY_CMD_LED_OFF;
> > + } else {
> > + volt = TM2_TOUCHKEY_LED_VOLTAGE_MAX;
> > + data = TM2_TOUCHKEY_CMD_LED_ON;
> > + }
> > +
> > + regulator_set_voltage(samsung_touchkey->regulator_vdd, volt, volt);
> > + i2c_smbus_write_byte_data(samsung_touchkey->client,
> > + TM2_TOUCHKEY_BASE_REG, data);
> > +}
> > +
> > +static int __tm2_touchkey_power_onoff(struct tm2_touchkey_data
> > + *samsung_touchkey, bool onoff)
> > +{
> > + int ret = 0;
> > +
> > + if (samsung_touchkey->power_onoff == onoff)
> > + return ret;
> > +
> > + if (onoff) {
> > + ret = regulator_enable(samsung_touchkey->regulator_vcc);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regulator_enable(samsung_touchkey->regulator_vdd);
> > + if (ret) {
> > + regulator_disable(samsung_touchkey->regulator_vcc);
> > + return ret;
> > + }
>
> I would add a comment about the sleep here.
Also "bulk" regulator API can be useful here.
>
> > + msleep(150);
> > + } else {
> > + int err;
> > +
> > + err = regulator_disable(samsung_touchkey->regulator_vcc);
> > + if (err)
> > + ret = err;
> > +
> > + err = regulator_disable(samsung_touchkey->regulator_vdd);
> > + if (err && !ret)
> > + ret = err;
> > + }
> > + samsung_touchkey->power_onoff = onoff;
> > +
> > + return ret;
> > +}
> > +
> > +static void tm2_touchkey_irq_work(struct work_struct *irq_work)
> > +{
> > + struct tm2_touchkey_data *samsung_touchkey =
> > + container_of(irq_work, struct tm2_touchkey_data, irq_work);
> > +
> > + if (!samsung_touchkey->pressed) {
> > + input_report_key(samsung_touchkey->input_dev, KEY_PHONE, 0);
> > + input_report_key(samsung_touchkey->input_dev, KEY_BACK, 0);
> > + } else {
> > + if (samsung_touchkey->keycode_type == TM2_TOUCHKEY_KEY_MENU)
> > + input_report_key(samsung_touchkey->input_dev,
> > + KEY_PHONE, 1);
> > + else
> > + input_report_key(samsung_touchkey->input_dev,
> > + KEY_BACK, 1);
> > + }
> > + input_sync(samsung_touchkey->input_dev);
> > +}
There is absolutely no reason for doing this in a work. Just call
input_report_key/input_sync from your threaded IRQ routine.
> > +
> > +static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid)
> > +{
> > + struct tm2_touchkey_data *samsung_touchkey = devid;
> > + u32 data;
> > +
> > + data = i2c_smbus_read_byte_data(samsung_touchkey->client,
> > + TM2_TOUCHKEY_KEYCODE_REG);
> > +
> > + if (data < 0) {
> > + dev_err(&samsung_touchkey->client->dev, "Failed to read i2c data\n");
> > + return IRQ_HANDLED;
> > + }
> > +
> > + samsung_touchkey->keycode_type = data & TM2_TOUCHKEY_BIT_KEYCODE;
> > + samsung_touchkey->pressed = !(data & TM2_TOUCHKEY_BIT_PRESS_EV);
> > +
> > + if (samsung_touchkey->keycode_type != TM2_TOUCHKEY_KEY_MENU &&
> > + samsung_touchkey->keycode_type != TM2_TOUCHKEY_KEY_BACK)
>
> Shouldn't at least a debug message be printed here so the user can
> know that an error occurred and a correct keycode was not received?
>
> > + return IRQ_HANDLED;
> > +
> > + schedule_work(&samsung_touchkey->irq_work);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int tm2_touchkey_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct tm2_touchkey_data *samsung_touchkey;
> > + int ret;
> > +
> > + ret = i2c_check_functionality(client->adapter,
> > + I2C_FUNC_SMBUS_BYTE |
> > + I2C_FUNC_SMBUS_BYTE_DATA);
> > + if (!ret) {
> > + dev_err(&client->dev, "No I2C functionality found\n");
> > + return -ENODEV;
> > + }
> > +
> > + samsung_touchkey = devm_kzalloc(&client->dev,
> > + sizeof(struct tm2_touchkey_data), GFP_KERNEL);
sizeof(*touchkey)
> > +
> > + if (!samsung_touchkey) {
> > + dev_err(&client->dev, "Failed to allocate memory.\n");
> > + return -ENOMEM;
> > + }
> > +
> > + samsung_touchkey->client = client;
> > + i2c_set_clientdata(client, samsung_touchkey);
> > + INIT_WORK(&samsung_touchkey->irq_work, tm2_touchkey_irq_work);
Work is not needed.
> > +
> > + /* regulator */
> > + samsung_touchkey->regulator_vcc =
> > + devm_regulator_get(&client->dev, "vcc");
> > + if (IS_ERR(samsung_touchkey->regulator_vcc)) {
> > + dev_err(&client->dev, "Failed to get vcc regulator\n");
> > + return PTR_ERR(samsung_touchkey->regulator_vcc);
> > + }
> > +
> > + samsung_touchkey->regulator_vdd =
> > + devm_regulator_get(&client->dev, "vdd");
> > + if (IS_ERR(samsung_touchkey->regulator_vdd)) {
> > + dev_err(&client->dev, "Failed to get vdd regulator\n");
> > + return PTR_ERR(samsung_touchkey->regulator_vcc);
> > + }
devm_regulator_bulk_get
> > +
> > + /* power */
> > + ret = tm2_touchkey_power_enable(samsung_touchkey);
> > + if (ret) {
> > + dev_err(&client->dev, "Failed to enable power\n");
> > + return ret;
> > + }
You need to install devm action (devm_add_action_or_reset) to disable
power when unloading driver (or if initialization fails).
> > +
> > + /* irq */
> > + ret = devm_request_threaded_irq(&client->dev,
> > + client->irq, NULL,
> > + tm2_touchkey_irq_handler,
> > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
I'd rather we rely on IRQ trigger from DT, so just use IRQF_PNESHOT
here.
I'd rather you allocated input device before requesting IRQ. The
registering input device is fine to do after getting IRQ.
> > + TM2_TOUCHKEY_DEV_NAME,
> > + samsung_touchkey);
> > + if (ret) {
> > + dev_err(&client->dev, "Failed to request threaded irq\n");
> > + return ret;
> > + }
> > +
> > + /* input device */
> > + samsung_touchkey->input_dev = devm_input_allocate_device(&client->dev);
> > + if (!samsung_touchkey->input_dev) {
> > + dev_err(&client->dev, "Failed to alloc input device.\n");
> > + return -ENOMEM;
> > + }
> > + samsung_touchkey->input_dev->name = TM2_TOUCHKEY_DEV_NAME;
> > + samsung_touchkey->input_dev->id.bustype = BUS_I2C;
> > + samsung_touchkey->input_dev->dev.parent = &client->dev;
No need to set parent when using devm_input_allocate_device(), it is
done automatically.
> > +
> > + set_bit(EV_KEY, samsung_touchkey->input_dev->evbit);
> > + set_bit(KEY_PHONE, samsung_touchkey->input_dev->keybit);
> > + set_bit(KEY_BACK, samsung_touchkey->input_dev->keybit);
Just do
input_set_capability(touchkey->input_dev, EV_KEY, KEY_PHONE);
input_set_capability(touchkey->input_dev, EV_KEY, KEY_BACK);
> > + input_set_drvdata(samsung_touchkey->input_dev, samsung_touchkey);
> > +
> > + ret = input_register_device(samsung_touchkey->input_dev);
> > + if (ret) {
> > + dev_err(&client->dev, "Failed to register input device.\n");
> > + return ret;
> > + }
> > +
> > + /* led device */
> > + samsung_touchkey->led_dev.name = TM2_TOUCHKEY_DEV_NAME;
> > + samsung_touchkey->led_dev.brightness = LED_FULL;
> > + samsung_touchkey->led_dev.max_brightness = LED_FULL;
> > + samsung_touchkey->led_dev.brightness_set =
> > + tm2_touchkey_led_brightness_set;
> > +
> > + ret = devm_led_classdev_register(&client->dev,
> > + &samsung_touchkey->led_dev);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "Failed to register touchkey led\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void tm2_touchkey_shutdown(struct i2c_client *client)
> > +{
This is something not normally done. Does your platform really keep
rails on when AP is off?
> > + struct tm2_touchkey_data *samsung_touchkey =
> > + i2c_get_clientdata(client);
> > + int ret;
> > +
> > + disable_irq(client->irq);
> > + ret = tm2_touchkey_power_disable(samsung_touchkey);
> > + if (ret)
> > + dev_err(&client->dev, "Failed to disable power\n");
> > +}
> > +
> > +static int tm2_touchkey_suspend(struct device *dev)
__maybe_unused
> > +{
> > + struct tm2_touchkey_data *samsung_touchkey = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + disable_irq(samsung_touchkey->client->irq);
> > + ret = tm2_touchkey_power_disable(samsung_touchkey);
> > + if (ret)
> > + dev_err(dev, "Failed to disable power\n");
> > +
> > + return ret;
> > +}
>
> These two functions are basically the same, can you factor it out?
>
> > +
> > +static int tm2_touchkey_resume(struct device *dev)
__maybe_unused
> > +{
> > + struct tm2_touchkey_data *samsung_touchkey = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + enable_irq(samsung_touchkey->client->irq);
> > + ret = tm2_touchkey_power_enable(samsung_touchkey);
> > + if (ret)
> > + dev_err(dev, "Failed to enable power\n");
> > +
> > + return ret;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(tm2_touchkey_pm_ops, tm2_touchkey_suspend,
> > + tm2_touchkey_resume);
> > +
> > +static const struct i2c_device_id tm2_touchkey_id_table[] = {
> > + {TM2_TOUCHKEY_DEV_NAME, 0},
> > + {},
> > +};
> > +
>
> You need a MODULE_DEVICE_TABLE(i2c, tm2_touchkey_id_table) here so the
> module can be autoloaded when the device is registered.
>
> > +static const struct of_device_id tm2_touchkey_of_match[] = {
> > + {.compatible = "samsung,tm2-touchkey",},
> > + {},
> > +};
> > +
>
> Here a MODULE_DEVICE_TABLE(of, tm2_touchkey_of_match) is not strictly
> needed since the I2C core always reports MODALIAS of the form i2c:<dev>
> but still is good to have so the I2C core can be fixed at some point.
Yes, please add MODULE_DEVICE_TABLE(of, ...).
>
> > +static struct i2c_driver tm2_touchkey_driver = {
> > + .driver = {
> > + .name = TM2_TOUCHKEY_DEV_NAME,
> > + .pm = &tm2_touchkey_pm_ops,
> > + .of_match_table = of_match_ptr(tm2_touchkey_of_match),
> > + },
> > + .probe = tm2_touchkey_probe,
> > + .shutdown = tm2_touchkey_shutdown,
> > + .id_table = tm2_touchkey_id_table,
> > +};
> > +
> > +module_i2c_driver(tm2_touchkey_driver);
> > +
> > +MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
> > +MODULE_AUTHOR("Jaechul Lee <jcsing.lee@samsung.com>");
> > +MODULE_DESCRIPTION("Samsung touchkey driver");
> > +MODULE_LICENSE("GPL v2");
> >
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets
From: Bjorn Andersson @ 2017-01-03 19:24 UTC (permalink / raw)
To: Vivek Gautam
Cc: Stephen Boyd, robh+dt, kishon,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland,
Srinivas Kandagatla, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAFp+6iF0FQjt3bt1d_HjYmpMb8cTkg+BudoNR7yzThd+EgZfQg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed 28 Dec 23:39 PST 2016, Vivek Gautam wrote:
> >> + *
> >> + */
> >> +static int phy_pipe_clk_register(struct qcom_qmp_phy *qphy, int id)
> >> +{
> >> + char clk_name[MAX_PROP_NAME];
> >
> > I'm not sure MAX_PROP_NAME is the same as some max clk name but
> > ok. We should be able to calculate that the maximum is length of
> > usb3_phy_pipe_clk_src for now though?
>
> Yea, i thought of using the same macro, considering that it provides
> 32 characters :-)
> Will rather use the length of usb3_phy_pipe_clk_src for now. May be
> #define MAX_CLK_NAME 24
>
Using a macro indicates that the size have some global meaning, but as
far as I can see it doesn't. Just write 24 and you will have enough
space for 100k pcie pipe clocks. And then use sizeof(clk_name) below.
> >
> >> + struct clk *clk;
> >> +
> >> + memset(&clk_name, 0, sizeof(clk_name));
There's no point in clearing clk_name, as it will be overwritten
and nul-terminated below.
> >> + switch (qphy->cfg->type) {
> >> + case PHY_TYPE_USB3:
> >> + snprintf(clk_name, MAX_PROP_NAME, "usb3_phy_pipe_clk_src");
> >> + break;
> >> + case PHY_TYPE_PCIE:
> >> + snprintf(clk_name, MAX_PROP_NAME, "pcie_%d_pipe_clk_src", id);
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +
> >> + /* controllers using QMP phys use 125MHz pipe clock interface */
> >> + clk = clk_register_fixed_rate(qphy->dev, clk_name, NULL, 0, 125000000);
Regards,
Bjorn
--
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] Documetation: binding: modify the exynos5440 pcie binding
From: Rob Herring @ 2017-01-03 19:51 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, mark.rutland-5wv7dgnIgG8,
kgene-DgEjT+Ai2ygdnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A,
kishon-l0cyMroinI0, jingoohan1-Re5JQEeQqe8AvxtiuMwx3w,
vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ,
pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ,
alim.akhtar-Sze3O3UU22JBDgjK7y7TUQ, cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20161228103454.26467-4-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Wed, Dec 28, 2016 at 07:34:53PM +0900, Jaehoon Chung wrote:
> According to using PHY framework, modified the exynos5440-pcie binding.
> And use "config" property to follow the designware-pcie binding.
You are breaking compatibility here. Please state that and why it is
okay to do so.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> .../bindings/pci/samsung,exynos5440-pcie.txt | 29 +++++++++++++---------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> index 4f9d23d..51f6214 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> @@ -5,10 +5,15 @@ and thus inherits all the common properties defined in designware-pcie.txt.
>
> Required properties:
> - compatible: "samsung,exynos5440-pcie"
> -- reg: base addresses and lengths of the pcie controller,
> - the phy controller, additional register for the phy controller.
> +- reg: base addresses and lengths of the pcie controller
> - interrupts: A list of interrupt outputs for level interrupt,
> pulse interrupt, special interrupt.
> +- phys: From PHY binding. Phandle for the Generic PHY.
> + Refer to Documentation/devicetree/bindings/phy/samsung-phy.txt
> +- phy-names: Must be "pcie-phy".
-names is pointless for a single element.
> +
> +Other common properties refer to
> + Documentation/devicetree/binding/pci/designware-pcie.txt
>
> Example:
>
> @@ -16,18 +21,18 @@ SoC specific DT Entry:
>
> pcie@290000 {
> compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> - reg = <0x290000 0x1000
> - 0x270000 0x1000
> - 0x271000 0x40>;
> + reg = <0x290000 0x1000>, <0x40000000 0x100>;
256 bytes of config space?
> + reg-names = "elbi", "config";
> interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
> clocks = <&clock 28>, <&clock 27>;
> clock-names = "pcie", "pcie_bus";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> - ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000 /* configuration space */
> - 0x81000000 0 0 0x40001000 0 0x00010000 /* downstream I/O */
> + ranges = <0x81000000 0 0 0x40001000 0 0x00010000 /* downstream I/O */
> 0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
> + phys = <&pcie_phy0>;
> + phy-names = "pcie-phy";
> #interrupt-cells = <1>;
> interrupt-map-mask = <0 0 0 0>;
> interrupt-map = <0 0 0 0 &gic GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> @@ -36,17 +41,17 @@ SoC specific DT Entry:
>
> pcie@2a0000 {
> compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
> - reg = <0x2a0000 0x1000
> - 0x272000 0x1000
> - 0x271040 0x40>;
> + reg = <0x2a0000 0x1000>, <0x60000000 0x1000>;
> + reg-names = "elbi", "config";
> interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
> clocks = <&clock 29>, <&clock 27>;
> clock-names = "pcie", "pcie_bus";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> - ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000 /* configuration space */
> - 0x81000000 0 0 0x60001000 0 0x00010000 /* downstream I/O */
> + phys = <&pcie_phy1>;
> + phy-names = "pcie-phy";
> + ranges = <0x81000000 0 0 0x60001000 0 0x00010000 /* downstream I/O */
> 0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
> #interrupt-cells = <1>;
> interrupt-map-mask = <0 0 0 0>;
> --
> 2.10.2
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rob Herring @ 2017-01-03 19:55 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
Martin Blumenstingl, Felix Fietkau, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
In-Reply-To: <20161228155955.25518-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> This new file should be used for properties handled at higher level and
> so usable with all drivers.
Why is this needed? Where would this data normally come from?
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> new file mode 100644
> index 0000000..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.
Not relavent to a binding. Bindings describe h/w.
> +
> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> + reg = <0x0000 0 0 0 0>;
> + ieee80211-min-center-freq = <2437000>;
> + ieee80211-max-center-freq = <2457000>;
> +};
> --
> 2.10.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/4] ARM: dts: Add am335x-boneblack-wireless
From: Tony Lindgren @ 2017-01-03 20:11 UTC (permalink / raw)
To: Jason Kridner
Cc: Robert Nelson, linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Kridner
In-Reply-To: <CB5C8999-6CAE-41F6-85A6-D2A856BD8011-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Jason Kridner <jkridner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [161228 13:27]:
>
>
> > On Dec 27, 2016, at 11:58 AM, Robert Nelson <robertcnelson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > BeagleBone Black Wireless is clone of the BeagleBone Black (BBB) with the Ethernet
> > replaced by a TI wl1835 wireless module.
> >
> > This board can be indentified by the BWAx value after A335BNLT (BBB) in the at24 eeprom:
> > BWAx [aa 55 33 ee 41 33 33 35 42 4e 4c 54 42 57 41 35 |.U3.A335BNLTBWA5|]
>
> I believe the correct statement is BWxx, but BWBx reserves the option to be software incompatible in some way. My preference is to have it boot anyway, but I believe that is only dependent in the bootloader.
So does this patch need updating for that?
Regards,
Tony
--
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: [PATCHv2 net-next 01/16] dt-bindings: net: update Marvell PPv2 binding for PPv2.2 support
From: Rob Herring @ 2017-01-03 20:18 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
devicetree-u79uwXL29TY76Z2rM5mHXA, Ian Campbell, Pawel Moll,
Mark Rutland, Kumar Gala, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Nadav Haklai, Hanna Hawa,
Yehuda Yitschak,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stefan Chulski,
Marcin Wojtas
In-Reply-To: <1482943592-12556-2-git-send-email-thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Wed, Dec 28, 2016 at 05:46:17PM +0100, Thomas Petazzoni wrote:
> The Marvell PPv2 Device Tree binding was so far only used to describe
> the PPv2.1 network controller, used in the Marvell Armada 375.
>
> A new version of this IP block, PPv2.2 is used in the Marvell Armada
> 7K/8K processor. This commit extends the existing binding so that it can
> also be used to describe PPv2.2 hardware.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/net/marvell-pp2.txt | 66 ++++++++++++++++++----
> 1 file changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt
> index aa4f423..76071f3 100644
> --- a/Documentation/devicetree/bindings/net/marvell-pp2.txt
> +++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt
> @@ -1,17 +1,28 @@
> -* Marvell Armada 375 Ethernet Controller (PPv2)
> +* Marvell Armada 375 Ethernet Controller (PPv2.1)
> + Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
>
> Required properties:
>
> -- compatible: should be "marvell,armada-375-pp2"
> +- compatible: should be one of:
> + "marvell,armada-375-pp2"
> + "marvell,armada-7k-pp2"
> - reg: addresses and length of the register sets for the device.
> - Must contain the following register sets:
> + For "marvell,armada-375-pp2", must contain the following register
> + sets:
> - common controller registers
> - LMS registers
> - In addition, at least one port register set is required.
> -- clocks: a pointer to the reference clocks for this device, consequently:
> - - main controller clock
> - - GOP clock
> -- clock-names: names of used clocks, must be "pp_clk" and "gop_clk".
> + - one register area per Ethernet port
> + For "marvell,armda-7k-pp2", must contain the following register
> + sets:
> + - common controller registers
> + - per-port registers
> +
> +- clocks: pointers to the reference clocks for this device, consequently:
> + - main controller clock (for both armada-375-pp2 and armada-7k-pp2)
> + - GOP clock (for both armada-375-pp2 and armada-7k-pp2)
> + - MG clock (only for armada-7k-pp2)
> +- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and
> + "mg_clk" (the latter only for armada-7k-pp2).
>
> The ethernet ports are represented by subnodes. At least one port is
> required.
> @@ -19,8 +30,9 @@ required.
> Required properties (port):
>
> - interrupts: interrupt for the port
> -- port-id: should be '0' or '1' for ethernet ports, and '2' for the
> - loopback port
> +- port-id: ID of the port from the MAC point of view
> +- gop-port-id: only for marvell,armada-7k-pp2, ID of the port from the
> + GOP (Group Of Ports) point of view
What GOP is needs a better explanation. Why doesn't 375 need this?
> - phy-mode: See ethernet.txt file in the same directory
>
> Optional properties (port):
> @@ -31,7 +43,7 @@ Optional properties (port):
> then fixed link is assumed, and the 'fixed-link' property is
> mandatory.
>
> -Example:
> +Example for marvell,armada-375-pp2:
>
> ethernet@f0000 {
> compatible = "marvell,armada-375-pp2";
> @@ -59,3 +71,35 @@ ethernet@f0000 {
> phy-mode = "gmii";
> };
> };
> +
> +Example for marvell,armada-7k-pp2:
> +
> +cpm_ethernet: ethernet@0 {
> + compatible = "marvell,armada-7k-pp22";
> + reg = <0x0 0x100000>,
> + <0x100000 0x80000>;
> + clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>;
> + clock-names = "pp_clk", "gop_clk", "gp_clk";
> + status = "disabled";
Drop status from examples.
> +
> + eth0: eth@0 {
unit address requires a reg property. Or this can be 'eth0' instead.
> + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
> + port-id = <0>;
> + gop-port-id = <0>;
> + status = "disabled";
> + };
> +
> + eth1: eth@1 {
> + interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> + port-id = <1>;
> + gop-port-id = <2>;
> + status = "disabled";
> + };
> +
> + eth2: eth@2 {
> + interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> + port-id = <2>;
> + gop-port-id = <3>;
> + status = "disabled";
> + };
> +};
> --
> 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 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2017-01-03 20:20 UTC (permalink / raw)
To: Rob Herring
Cc: Kalle Valo,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Martin Blumenstingl, Felix Fietkau, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rafał Miłecki
In-Reply-To: <20170103195514.vf4psb6qw2bd2lcw@rob-hp-laptop>
On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>
> Why is this needed? Where would this data normally come from?
Vendors limit choice of channels at their web user interface level. I
want to do better and report proper channels directly at kernel level
instead of masking them in every possible configuration tool.
>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a proper
>> +net layer and don't require extra driver code.
>
> Not relavent to a binding. Bindings describe h/w.
Yes, Arend pointed it to me and I improved this Documentation file in
further versions. Could you review V4, please?
https://patchwork.kernel.org/patch/9494713/
--
Rafał
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline
From: Christopher Spinrath @ 2017-01-03 20:23 UTC (permalink / raw)
To: Shawn Guo, p.zabel
Cc: mark.rutland, devicetree, christopher.spinrath, linux, robh+dt,
grinberg, kernel, fabio.estevam, linux-arm-kernel
In-Reply-To: <27256207f2d84b1ca4b7dfc41a413fcc@rwthex-s2-a.rwth-ad.de>
Hi Shawn,
thanks for the review!
On 12/30/2016 03:27 PM, Shawn Guo wrote:
> On Fri, Dec 02, 2016 at 03:37:22PM +0100, christopher.spinrath@rwth-aachen.de wrote:
>> From: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>
>>
>> Apart from the already enabled Designware HDMI port, the Utilite Pro
>> has a second display pipeline which has the following shape:
>>
>> IPU1 DI0 --> Parallel display --> tfp410 rgb24 to DVI encoder
>> --> HDMI connector.
>> Enable support for it.
>>
>> In addition, since this pipeline is hardwired to IPU1, sever the link
>> between IPU1 and the SoC-internal Designware HDMI encoder forcing the
>> latter to be connected to IPU2 instead of IPU1. Otherwise, it is not
>> possible to drive both displays at high resolution due to the bandwidth
>> limitations of a single IPU.
>>
>> Signed-off-by: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>
>
> @Philipp, can you help review the changes?
>
>> ---
>>
>> Hi all,
>>
>> the removal of the link between IPU1 and the Designware HDMI encoder is the
>> result of a discussion I had with Philipp Zabel:
>>
>> https://lists.freedesktop.org/archives/dri-devel/2016-November/125399.html .
>>
>> Altough it is not possible to connect anything else to IPU1 on the Utilite, this
>> approach has at least one disadvantage: if the resolution is low enough such
>> that a single IPU can handle both displays then muxing both displays to IPU1
>> would reduce the power consumption.
>>
>> However, IMHO omitting the link IPU1 <--> DW HDMI is still the preferrable
>> solution since I'm not aware of any OS/driver that is capable of switching IPUs
>> or can handle the bandwidth limitation in a sane way. In particular, Linux is
>> unusable when both displays are supposed to be driven at high resolution and
>> both muxing options for the DW HDMI are available (this is not a userspace
>> issue; the system becomes almost unresponsive as soon as the kernel sets the
>> initial resolution).
>>
>> Cheers,
>> Christopher
>>
>> P.S.: this patch depends on the tfp410 bridge driver which has recently been
>> merged into drm-next.
>
> v4.10-rc1 has the driver, so the dependency is gone now, I guess.
That's correct.
>>
>> arch/arm/boot/dts/imx6q-utilite-pro.dts | 115 ++++++++++++++++++++++++++++++++
>> 1 file changed, 115 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6q-utilite-pro.dts b/arch/arm/boot/dts/imx6q-utilite-pro.dts
>> index 2200994..69bdd82 100644
>> --- a/arch/arm/boot/dts/imx6q-utilite-pro.dts
>> +++ b/arch/arm/boot/dts/imx6q-utilite-pro.dts
>> @@ -59,6 +59,33 @@
>> rtc1 = &snvs_rtc;
>> };
>>
>> + encoder {
>> + compatible = "ti,tfp410";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> +
>> + tfp410_in: endpoint {
>> + remote-endpoint = <¶llel_display_out>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> +
>> + tfp410_out: endpoint {
>> + remote-endpoint = <&hdmi_connector_in>;
>> + };
>> + };
>> + };
>> + };
>> +
>> gpio-keys {
>> compatible = "gpio-keys";
>> pinctrl-names = "default";
>> @@ -72,6 +99,19 @@
>> };
>> };
>>
>> + hdmi-connector {
>> + compatible = "hdmi-connector";
>> +
>
> The newline is unnecessary.
>
>> + type = "a";
>> + ddc-i2c-bus = <&i2c_dvi_ddc>;
>> +
>> + port {
>> + hdmi_connector_in: endpoint {
>> + remote-endpoint = <&tfp410_out>;
>> + };
>> + };
>> + };
>> +
>> i2cmux {
>> compatible = "i2c-mux-gpio";
>> pinctrl-names = "default";
>> @@ -105,8 +145,46 @@
>> #size-cells = <0>;
>> };
>> };
>> +
>> + parallel-display {
>> + compatible = "fsl,imx-parallel-display";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_ipu1>;
>> +
>
> Ditto
>
> I can fix them up if I get a Reviewed-by tag from Philipp on this
> version.
Thanks. I'll bear that in mind for future patches (and, of course, I
will fix them up myself if I have to send a v2).
Cheers,
Christopher
> Shawn
>
>> + interface-pix-fmt = "rgb24";
>> +
>> + port@0 {
>> + reg = <0>;
>> +
>> + parallel_display_in: endpoint {
>> + remote-endpoint = <&ipu1_di0_disp0>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> +
>> + parallel_display_out: endpoint {
>> + remote-endpoint = <&tfp410_in>;
>> + };
>> + };
>> + };
>> };
>>
>> +/*
>> + * A single IPU is not able to drive both display interfaces available on the
>> + * Utilite Pro at high resolution due to its bandwidth limitation. Since the
>> + * tfp410 encoder is wired up to IPU1, sever the link between IPU1 and the
>> + * SoC-internal Designware HDMI encoder forcing the latter to be connected to
>> + * IPU2 instead of IPU1.
>> + */
>> +/delete-node/&ipu1_di0_hdmi;
>> +/delete-node/&hdmi_mux_0;
>> +/delete-node/&ipu1_di1_hdmi;
>> +/delete-node/&hdmi_mux_1;
>> +
>> &hdmi {
>> ddc-i2c-bus = <&i2c2>;
>> status = "okay";
>> @@ -151,6 +229,39 @@
>> >;
>> };
>>
>> + pinctrl_ipu1: ipu1grp {
>> + fsl,pins = <
>> + MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x38
>> + MX6QDL_PAD_DI0_PIN15__IPU1_DI0_PIN15 0x38
>> + MX6QDL_PAD_DI0_PIN2__IPU1_DI0_PIN02 0x38
>> + MX6QDL_PAD_DI0_PIN3__IPU1_DI0_PIN03 0x38
>> + MX6QDL_PAD_DISP0_DAT0__IPU1_DISP0_DATA00 0x38
>> + MX6QDL_PAD_DISP0_DAT1__IPU1_DISP0_DATA01 0x38
>> + MX6QDL_PAD_DISP0_DAT2__IPU1_DISP0_DATA02 0x38
>> + MX6QDL_PAD_DISP0_DAT3__IPU1_DISP0_DATA03 0x38
>> + MX6QDL_PAD_DISP0_DAT4__IPU1_DISP0_DATA04 0x38
>> + MX6QDL_PAD_DISP0_DAT5__IPU1_DISP0_DATA05 0x38
>> + MX6QDL_PAD_DISP0_DAT6__IPU1_DISP0_DATA06 0x38
>> + MX6QDL_PAD_DISP0_DAT7__IPU1_DISP0_DATA07 0x38
>> + MX6QDL_PAD_DISP0_DAT8__IPU1_DISP0_DATA08 0x38
>> + MX6QDL_PAD_DISP0_DAT9__IPU1_DISP0_DATA09 0x38
>> + MX6QDL_PAD_DISP0_DAT10__IPU1_DISP0_DATA10 0x38
>> + MX6QDL_PAD_DISP0_DAT11__IPU1_DISP0_DATA11 0x38
>> + MX6QDL_PAD_DISP0_DAT12__IPU1_DISP0_DATA12 0x38
>> + MX6QDL_PAD_DISP0_DAT13__IPU1_DISP0_DATA13 0x38
>> + MX6QDL_PAD_DISP0_DAT14__IPU1_DISP0_DATA14 0x38
>> + MX6QDL_PAD_DISP0_DAT15__IPU1_DISP0_DATA15 0x38
>> + MX6QDL_PAD_DISP0_DAT16__IPU1_DISP0_DATA16 0x38
>> + MX6QDL_PAD_DISP0_DAT17__IPU1_DISP0_DATA17 0x38
>> + MX6QDL_PAD_DISP0_DAT18__IPU1_DISP0_DATA18 0x38
>> + MX6QDL_PAD_DISP0_DAT19__IPU1_DISP0_DATA19 0x38
>> + MX6QDL_PAD_DISP0_DAT20__IPU1_DISP0_DATA20 0x38
>> + MX6QDL_PAD_DISP0_DAT21__IPU1_DISP0_DATA21 0x38
>> + MX6QDL_PAD_DISP0_DAT22__IPU1_DISP0_DATA22 0x38
>> + MX6QDL_PAD_DISP0_DAT23__IPU1_DISP0_DATA23 0x38
>> + >;
>> + };
>> +
>> pinctrl_uart2: uart2grp {
>> fsl,pins = <
>> MX6QDL_PAD_GPIO_7__UART2_TX_DATA 0x1b0b1
>> @@ -194,6 +305,10 @@
>> };
>> };
>>
>> +&ipu1_di0_disp0 {
>> + remote-endpoint = <¶llel_display_in>;
>> +};
>> +
>> &pcie {
>> pcie@0,0 {
>> reg = <0x000000 0 0 0 0>;
>> --
>> 2.10.2
>>
^ 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