devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v7 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-12-23 12:41 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Rob Herring, Maxime Coquelin, Alexandre Torgue,
	Linus Walleij, Patrice Chotard, linux, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20161223090019.a3jkhpb3abgjqi55@pengutronix.de>

Hi Uwe,

Thanks for your comments.
Please see below my answers and one question regarding duty cycle:

2016-12-23 10:00 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
>
> On Thu, Dec 22, 2016 at 02:35:01PM +0100, M'boumba Cedric Madianga wrote:
>> This patch adds support for the STM32F4 I2C controller.
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> ---
>>  drivers/i2c/busses/Kconfig       |  10 +
>>  drivers/i2c/busses/Makefile      |   1 +
>>  drivers/i2c/busses/i2c-stm32f4.c | 896 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 907 insertions(+)
>>  create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 0cdc844..2719208 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -886,6 +886,16 @@ config I2C_ST
>>         This driver can also be built as module. If so, the module
>>         will be called i2c-st.
>>
>> +config I2C_STM32F4
>> +     tristate "STMicroelectronics STM32F4 I2C support"
>> +     depends on ARCH_STM32 || COMPILE_TEST
>> +     help
>> +       Enable this option to add support for STM32 I2C controller embedded
>> +       in STM32F4 SoCs.
>> +
>> +       This driver can also be built as module. If so, the module
>> +       will be called i2c-stm32f4.
>> +
>>  config I2C_STU300
>>       tristate "ST Microelectronics DDC I2C interface"
>>       depends on MACH_U300
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 1c1bac8..a2c6ff5 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_SH_MOBILE) += i2c-sh_mobile.o
>>  obj-$(CONFIG_I2C_SIMTEC)     += i2c-simtec.o
>>  obj-$(CONFIG_I2C_SIRF)               += i2c-sirf.o
>>  obj-$(CONFIG_I2C_ST)         += i2c-st.o
>> +obj-$(CONFIG_I2C_STM32F4)    += i2c-stm32f4.o
>>  obj-$(CONFIG_I2C_STU300)     += i2c-stu300.o
>>  obj-$(CONFIG_I2C_SUN6I_P2WI) += i2c-sun6i-p2wi.o
>>  obj-$(CONFIG_I2C_TEGRA)              += i2c-tegra.o
>> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
>> new file mode 100644
>> index 0000000..ca11dee
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-stm32f4.c
>> @@ -0,0 +1,896 @@
>> +/*
>> + * Driver for STMicroelectronics STM32 I2C controller
>> + *
>> + * This I2C controller is described in the STM32F429/439 Soc reference manual.
>> + * Please see below a link to the documentation:
>> + * http://www.st.com/resource/en/reference_manual/DM00031020.pdf
>> + *
>> + * Copyright (C) M'boumba Cedric Madianga 2016
>> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> + *
>> + * This driver is based on i2c-st.c
>> + *
>> + * License terms:  GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +/* STM32F4 I2C offset registers */
>> +#define STM32F4_I2C_CR1                      0x00
>> +#define STM32F4_I2C_CR2                      0x04
>> +#define STM32F4_I2C_DR                       0x10
>> +#define STM32F4_I2C_SR1                      0x14
>> +#define STM32F4_I2C_SR2                      0x18
>> +#define STM32F4_I2C_CCR                      0x1C
>> +#define STM32F4_I2C_TRISE            0x20
>> +#define STM32F4_I2C_FLTR             0x24
>> +
>> +/* STM32F4 I2C control 1*/
>> +#define STM32F4_I2C_CR1_SWRST                BIT(15)
>> +#define STM32F4_I2C_CR1_POS          BIT(11)
>> +#define STM32F4_I2C_CR1_ACK          BIT(10)
>> +#define STM32F4_I2C_CR1_STOP         BIT(9)
>> +#define STM32F4_I2C_CR1_START                BIT(8)
>> +#define STM32F4_I2C_CR1_PE           BIT(0)
>> +
>> +/* STM32F4 I2C control 2 */
>> +#define STM32F4_I2C_CR2_FREQ_MASK    GENMASK(5, 0)
>> +#define STM32F4_I2C_CR2_FREQ(n)              (((n) & STM32F4_I2C_CR2_FREQ_MASK))
>
>                 ((n) & STM32F4_I2C_CR2_FREQ_MASK)
>
> should be enough.
You are right. I will fix it in the V8.

>
>> +#define STM32F4_I2C_CR2_ITBUFEN              BIT(10)
>> +#define STM32F4_I2C_CR2_ITEVTEN              BIT(9)
>> +#define STM32F4_I2C_CR2_ITERREN              BIT(8)
>> +#define STM32F4_I2C_CR2_IRQ_MASK     (STM32F4_I2C_CR2_ITBUFEN | \
>> +                                      STM32F4_I2C_CR2_ITEVTEN | \
>> +                                      STM32F4_I2C_CR2_ITERREN)
>> +
>> +/* STM32F4 I2C Status 1 */
>> +#define STM32F4_I2C_SR1_AF           BIT(10)
>> +#define STM32F4_I2C_SR1_ARLO         BIT(9)
>> +#define STM32F4_I2C_SR1_BERR         BIT(8)
>> +#define STM32F4_I2C_SR1_TXE          BIT(7)
>> +#define STM32F4_I2C_SR1_RXNE         BIT(6)
>> +#define STM32F4_I2C_SR1_BTF          BIT(2)
>> +#define STM32F4_I2C_SR1_ADDR         BIT(1)
>> +#define STM32F4_I2C_SR1_SB           BIT(0)
>> +#define STM32F4_I2C_SR1_ITEVTEN_MASK (STM32F4_I2C_SR1_BTF | \
>> +                                      STM32F4_I2C_SR1_ADDR | \
>> +                                      STM32F4_I2C_SR1_SB)
>> +#define STM32F4_I2C_SR1_ITBUFEN_MASK (STM32F4_I2C_SR1_TXE | \
>> +                                      STM32F4_I2C_SR1_RXNE)
>> +#define STM32F4_I2C_SR1_ITERREN_MASK (STM32F4_I2C_SR1_AF | \
>> +                                      STM32F4_I2C_SR1_ARLO | \
>> +                                      STM32F4_I2C_SR1_BERR)
>> +
>> +/* STM32F4 I2C Status 2 */
>> +#define STM32F4_I2C_SR2_BUSY         BIT(1)
>> +
>> +/* STM32F4 I2C Control Clock */
>> +#define STM32F4_I2C_CCR_CCR_MASK     GENMASK(11, 0)
>> +#define STM32F4_I2C_CCR_CCR(n)               (((n) & STM32F4_I2C_CCR_CCR_MASK))
>
> ditto
ok

>
>> +#define STM32F4_I2C_CCR_FS           BIT(15)
>> +#define STM32F4_I2C_CCR_DUTY         BIT(14)
>> +
>> +/* STM32F4 I2C Trise */
>> +#define STM32F4_I2C_TRISE_VALUE_MASK GENMASK(5, 0)
>> +#define STM32F4_I2C_TRISE_VALUE(n)   (((n) & STM32F4_I2C_TRISE_VALUE_MASK))
>> +
>> +/* STM32F4 I2C Filter */
>> +#define STM32F4_I2C_FLTR_DNF_MASK    GENMASK(3, 0)
>> +#define STM32F4_I2C_FLTR_DNF(n)              (((n) & STM32F4_I2C_FLTR_DNF_MASK))
>> +#define STM32F4_I2C_FLTR_ANOFF               BIT(4)
>> +
>> +#define STM32F4_I2C_MIN_FREQ         2U
>> +#define STM32F4_I2C_MAX_FREQ         42U
>> +#define HZ_TO_MHZ                    1000000
>> +
>> +enum stm32f4_i2c_speed {
>> +     STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
>> +     STM32F4_I2C_SPEED_FAST, /* 400 kHz */
>> +     STM32F4_I2C_SPEED_END,
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_timings - per-Mode tuning parameters
>> + * @duty: Fast mode duty cycle
>> + * @scl_period: SCL low/high period in microsecond
>> + * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
>> + * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
>
> s/Khz/ kHz/
Good point. Thanks

>
>> + */
>> +struct stm32f4_i2c_timings {
>> +     u32 duty;
>> +     u32 scl_period;
>> +     u32 mul_ccr;
>> +     u32 min_ccr;
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_msg - client specific data
>> + * @addr: 8-bit slave addr, including r/w bit
>> + * @count: number of bytes to be transferred
>> + * @buf: data buffer
>> + * @result: result of the transfer
>> + * @stop: last I2C msg to be sent, i.e. STOP to be generated
>> + */
>> +struct stm32f4_i2c_msg {
>> +     u8      addr;
>> +     u32     count;
>> +     u8      *buf;
>> +     int     result;
>> +     bool    stop;
>
> You bought the argument about alignment of = in stm32f4_i2c_driver. The
> same logic applies here.
ok

>
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_dev - private data of the controller
>> + * @adap: I2C adapter for this controller
>> + * @dev: device for this controller
>> + * @base: virtual memory area
>> + * @complete: completion of I2C message
>> + * @clk: hw i2c clock
>> + * speed: I2C clock frequency of the controller. Standard or Fast only supported
>> + * @msg: I2C transfer information
>> + */
>> +struct stm32f4_i2c_dev {
>> +     struct i2c_adapter              adap;
>> +     struct device                   *dev;
>> +     void __iomem                    *base;
>> +     struct completion               complete;
>> +     struct clk                      *clk;
>> +     int                             speed;
>> +     struct stm32f4_i2c_msg          msg;
>> +};
>
> ditto
ok

>
>> +
>> +/*
>> + * In standard mode:
>> + * SCL high period = SCL low period = CCR * I2C CLK period
>> + * So, CCR = SCL period * I2C CLK frequency
>
> is "SCL period" the same as "SCL low period"? I2C CLK frequency is the
> input clk? If so, it's confusing that it has I2C in its name. The
> reference manual calls it PCLK1 (parent clock?).
SCL high period = SCL low period
I2C CLK frequency = I2C parent clock frequency so I will add this
precision in the V8

>
>> + * In fast mode:
>> + * DUTY = 0: Fast mode tlow/thigh = 2
>> + * DUTY = 1: Fast mode tlow/thigh = 16/9
>> + * If Duty = 0; SCL high period = 1  * CCR * I2C CLK period
>> + *           SCL low period  = 2  * CCR * I2C CLK period
>> + * If Duty = 1; SCL high period = 9  * CCR * I2C CLK period
>> + *           SCL low period  = 16 * CCR * I2C CLK period
>
> I'd drop the first two lines about the proportions
>
>> + *
>> + * Note that Duty has to bet set to reach 400khz in Fast mode
>
> s/khz/ kHz/
Ok. Thanks.

>
> I don't understand why DUTY is required to reach 400 kHz. Given a parent
> freq of 30 MHz, with CCR = 25 and DUTY = 0 we have:
>
>         t_high = 25 * 33.333 ns = 833.333 ns
>         t_low = 2 * 25 * 33.333 ns = 1666.667 ns
>
> then t_high and t_low satisfy the i2c bus specification
> (t_low > 1300 ns, t_high > 600 ns) and we have t_low + t_high = 2500 ns
> = 1 / 400 kHz.
>
> Where is the error?
Hum ok you are right. I was a bad interpretation of the datasheet.
So now it is clearer. Thanks for that.
I will correct and improve my comments in the V8.

>
>> + * So, in order to cover both SCL high/low with Duty = 1,
>> + * CCR = 16 * SCL period * I2C CLK frequency
>
> I don't get that. Actually you need to use low + high, so
> CCR = parentrate / (25 * 400 kHz), right?
With your new inputs above, I think I could use a simpler implementation:
CCR = scl_high_period * parent_rate
with scl_high_period = 5 µs in standard mode to reach 100khz
and scl_high_period = 1 µs in fast mode to reach 400khz with 1/2 or
16/9 duty cycle.
So, I am wondering if I have to let the customer setting the duty
cycle in the DT for example with "st,duty=0" or "st,duty=1" property
(0 for 1/2 and 1 for 16/9).
Or perhaps the best option it to use a default value. What is your
feeling regarding this point ?

>
>> + *
>> + * Please note that the minimum allowed value is 0x04, except in FAST DUTY mode
>> + * where the minimum allowed value is 0x01
>> + */
>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>> +     [STM32F4_I2C_SPEED_STANDARD] = {
>> +             .mul_ccr                = 1,
>> +             .min_ccr                = 4,
>> +             .duty                   = 0,
>> +             .scl_period             = 5,
>> +     },
>> +     [STM32F4_I2C_SPEED_FAST] = {
>> +             .mul_ccr                = 16,
>> +             .min_ccr                = 1,
>> +             .duty                   = 1,
>> +             .scl_period             = 2,
>> +     },
>> +};
>> +
>> +static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
>> +{
>> +     writel_relaxed(readl_relaxed(reg) | mask, reg);
>> +}
>> +
>> +static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
>> +{
>> +     writel_relaxed(readl_relaxed(reg) & ~mask, reg);
>> +}
>> +
>> +static void stm32f4_i2c_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +     u32 val;
>> +
>> +     val = readl_relaxed(reg);
>> +     writel_relaxed(val | STM32F4_I2C_CR1_SWRST, reg);
>> +     writel_relaxed(val, reg);
>> +}
>> +
>> +static void stm32f4_i2c_disable_irq(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
>> +}
>> +
>> +static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 clk_rate, cr2, freq;
>> +
>> +     /*
>> +      * The minimum allowed frequency is 2 MHz, the maximum frequency is
>> +      * limited by the maximum APB frequency 42 MHz
>> +      */
>> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
>> +     clk_rate = clk_get_rate(i2c_dev->clk);
>> +     freq = DIV_ROUND_UP(clk_rate, HZ_TO_MHZ);
>> +     freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
>> +     cr2 |= STM32F4_I2C_CR2_FREQ(freq);
>> +     writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
>
> Last round I suggested error checking here instead of silent clamping.
Ok

>
>> +}
>> +
>> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 trise, freq, cr2;
>> +
>> +     /*
>> +      * These bits must be programmed with the maximum SCL rise time given in
>> +      * the I2C bus specification, incremented by 1.
>> +      *
>> +      * In standard mode, the maximum allowed SCL rise time is 1000 ns.
>> +      * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
>> +      * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
>> +      * programmed with 09h.(1000 ns / 125 ns = 8 + 1)
>> +      * So, for I2C standard mode TRISE = FREQ[5:0] + 1
>> +      *
>> +      * In fast mode, the maximum allowed SCL rise time is 300 ns.
>> +      * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
>> +      * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
>> +      * programmed with 03h.(300 ns / 125 ns = 2 + 1)
>> +      * So, for I2C fast mode TRISE = FREQ[5:0] * 300 / 1000 + 1
>> +      */
>> +
>> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
>> +
>> +     if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
>> +             trise = freq + 1;
>> +     else
>> +             trise = freq * 300 / 1000 + 1;
>
> if freq is big such that freq * 300 overflows does this result in a
> wrong result, or does the compiler optimize correctly?
For sure the compiler will never exceeds u32 max value

>
>> +     writel_relaxed(STM32F4_I2C_TRISE_VALUE(trise),
>> +                    i2c_dev->base + STM32F4_I2C_TRISE);
>> +}
>> +
>> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
>> +     u32 cr2, ccr, freq, val;
>> +
>> +     ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
>> +     ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
>> +              STM32F4_I2C_CCR_CCR_MASK);
>> +
>> +     /*
>> +      * Please see the comments above regarding i2c_timings[] declaration
>> +      * to understand the below calculation
>> +      */
>> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
>> +     val = freq * t->scl_period * t->mul_ccr;
>> +     if (val < t->min_ccr)
>> +             val = t->min_ccr;
>> +     ccr |= STM32F4_I2C_CCR_CCR(val);
>> +
>> +     if (t->duty)
>> +             ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
>> +
>> +     writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
>> +}
>> +
>> +static void stm32f4_i2c_set_filter(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 filter;
>> +
>> +     /* Enable analog noise filter and disable digital noise filter */
>> +     filter = readl_relaxed(i2c_dev->base + STM32F4_I2C_FLTR);
>> +     filter &= ~(STM32F4_I2C_FLTR_ANOFF | STM32F4_I2C_FLTR_DNF_MASK);
>> +     writel_relaxed(filter, i2c_dev->base + STM32F4_I2C_FLTR);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_hw_config() - Prepare I2C block
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +
>> +     /* Disable I2C */
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
>> +
>> +     stm32f4_i2c_set_periph_clk_freq(i2c_dev);
>> +
>> +     stm32f4_i2c_set_rise_time(i2c_dev);
>> +
>> +     stm32f4_i2c_set_speed_mode(i2c_dev);
>> +
>> +     stm32f4_i2c_set_filter(i2c_dev);
>> +
>> +     /* Enable I2C */
>> +     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
>
> This is the first write to STM32F4_I2C_CR1, right? So the state from the
> bootloader leaks here. This probably works most of the time, but if it
> makes problems later, that's hard to debug. Also, what if the bootloader
> already did some i2c transfers and kept the PE bit 1? I read in the
> manual that PE must be 0 for some things. So this only works most of the
> time.
The first thing I do before configuring I2C device is to clear PE bit
in order to disable I2C.
In that way, all previous config will be erased by the new one.

>
>> +}
>> +
>> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 status;
>> +     int ret;
>> +
>> +     ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
>> +                                      status,
>> +                                      !(status & STM32F4_I2C_SR2_BUSY),
>> +                                      10, 1000);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "bus not free\n");
>
> drop error message please or degrade to dev_debug
ok I will use a dev_dbg message

>
>> +             ret = -EBUSY;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
>> + * @i2c_dev: Controller's private data
>> + * @byte: Data to write in the register
>> + */
>> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
>> +{
>> +     writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
>> + * @i2c_dev: Controller's private data
>> + *
>> + * This function fills the data register with I2C transfer buffer
>> + */
>> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +
>> +     stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
>> +     msg->count--;
>> +}
>> +
>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     u32 rbuf;
>> +
>> +     rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>> +     *msg->buf++ = rbuf & 0xff;
>> +     msg->count--;
>> +}
>> +
>> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     stm32f4_i2c_disable_irq(i2c_dev);
>> +
>> +     reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +     if (msg->stop)
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +     else
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> +     complete(&i2c_dev->complete);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     if (msg->count) {
>> +             stm32f4_i2c_write_msg(i2c_dev);
>> +             if (!msg->count) {
>> +                     /* Disable buffer interrupts for RXNE/TXE events */
>> +                     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> +             }
>> +     } else {
>> +             stm32f4_i2c_terminate_xfer(i2c_dev);
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     switch (msg->count) {
>> +     case 1:
>> +             stm32f4_i2c_disable_irq(i2c_dev);
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +             complete(&i2c_dev->complete);
>> +             break;
>> +     case 2:
>> +     case 3:
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> +             break;
>> +     default:
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
>> + * in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg;
>> +     u32 mask;
>> +     int i;
>> +
>> +     switch (msg->count) {
>> +     case 2:
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             /* Generate STOP or repeated Start */
>> +             if (msg->stop)
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +             else
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> +             /* Read two last data bytes */
>> +             for (i = 2; i > 0; i--)
>> +                     stm32f4_i2c_read_msg(i2c_dev);
>> +
>> +             /* Disable events and error interrupts */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +             mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>> +             stm32f4_i2c_clr_bits(reg, mask);
>> +
>> +             complete(&i2c_dev->complete);
>> +             break;
>> +     case 3:
>> +             /* Enable ACK and read data */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +             break;
>> +     default:
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
>> + * master receiver
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg;
>> +
>> +     switch (msg->count) {
>> +     case 0:
>> +             stm32f4_i2c_terminate_xfer(i2c_dev);
>> +             /* Clear ADDR flag */
>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             break;
>> +     case 1:
>> +             /*
>> +              * Single byte reception:
>> +              * Enable NACK, clear ADDR flag and generate STOP or RepSTART
>> +              */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             if (msg->stop)
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +             else
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +             break;
>> +     case 2:
>> +             /*
>> +              * 2-byte reception:
>> +              * Enable NACK and set POS
>> +              */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             break;
>> +
>> +     default:
>> +             /* N-byte reception: Enable ACK */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             break;
>> +     }
>> +}
>
> This is still not really understandable.
I have already added some comments from datasheet to explain the
different cases.
I don't see how I could be more understandable as it is clearly the
hardware way of working...

>
>> +
>> +/**
>> + * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
>> + * @irq: interrupt number
>> + * @data: Controller's private data
>> + */
>> +static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = data;
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg;
>> +     u32 status, possible_status, ien;
>> +     int flag;
>> +
>> +     ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     ien &= STM32F4_I2C_CR2_IRQ_MASK;
>> +     possible_status = 0;
>
> This can already be done when declaring possible_status.
Ok.

>
>> +
>> +     /* Check possible status combinations */
>> +     if (ien & STM32F4_I2C_CR2_ITEVTEN) {
>> +             possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
>> +             if (ien & STM32F4_I2C_CR2_ITBUFEN)
>> +                     possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
>> +     }
>> +
>> +     status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>> +
>> +     if (!(status & possible_status)) {
>> +             dev_dbg(i2c_dev->dev,
>> +                     "spurious evt irq (status=0x%08x, ien=0x%08x)\n",
>> +                     status, ien);
>> +             return IRQ_NONE;
>> +     }
>> +
>> +     while (status & possible_status) {
>> +             /* Use __fls() to check error bits first */
>> +             flag = __fls(status & possible_status);
>> +
>> +             switch (1 << flag) {
>> +             case STM32F4_I2C_SR1_SB:
>> +                     stm32f4_i2c_write_byte(i2c_dev, msg->addr);
>> +                     break;
>> +
>> +             case STM32F4_I2C_SR1_ADDR:
>> +                     if (msg->addr & I2C_M_RD)
>> +                             stm32f4_i2c_handle_rx_addr(i2c_dev);
>> +                     else
>> +                             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +
>> +                     /* Enable buffer interrupts for RXNE/TXE events */
>> +                     reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> +                     possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
>> +                     break;
>> +
>> +             case STM32F4_I2C_SR1_BTF:
>> +                     if (msg->addr & I2C_M_RD)
>> +                             stm32f4_i2c_handle_rx_btf(i2c_dev);
>> +                     else
>> +                             stm32f4_i2c_handle_write(i2c_dev);
>> +                     break;
>> +
>> +             case STM32F4_I2C_SR1_TXE:
>> +                     stm32f4_i2c_handle_write(i2c_dev);
>> +                     break;
>> +
>> +             case STM32F4_I2C_SR1_RXNE:
>> +                     stm32f4_i2c_handle_read(i2c_dev);
>> +                     break;
>> +
>> +             default:
>> +                     dev_err(i2c_dev->dev,
>> +                             "evt irq unhandled: status=0x%08x)\n",
>> +                             status);
>> +                     return IRQ_NONE;
>> +             }
>> +             status &= ~(1 << flag);
>> +     }
>
> I wouldn't do this in a loop. Just do:
>
>         if (status & STM32F4_I2C_SR1_SB) {
>                 ...
>         }
>
>         if (status & ...) {
>
>         }
ok but I would prefer something like that:
flag = status & possible_status
if (flag & STM32F4_I2C_SR1_SB) {
...
}

if (flag & ...) {
}

>
> Then it's obvious by reading the code in which order they are handled
> without the need to check the definitions. Do you really need to jugle
> with possible_status?
I think I have to use possible_status as some events could occur
whereas the corresponding interrupt is disabled.
For example, for a 2 byte-reception, we don't have to take into accout
RXNE event so the corresponding interrupt is disabled.
If I don't check possible_status, I will call
stm32f4_i2c_handle_read() for nothing and generates unneeded registers
accesses.

>
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_isr_error() - Interrupt routine for I2C bus error
>> + * @irq: interrupt number
>> + * @data: Controller's private data
>> + */
>> +static irqreturn_t stm32f4_i2c_isr_error(int irq, void *data)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = data;
>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +     void __iomem *reg;
>> +     u32 status, possible_status, ien;
>> +     int flag;
>> +
>> +     ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     ien &= STM32F4_I2C_CR2_IRQ_MASK;
>> +     possible_status = 0;
>> +
>> +     /* Check possible status combinations */
>> +     if (ien & STM32F4_I2C_CR2_ITERREN)
>> +             possible_status = STM32F4_I2C_SR1_ITERREN_MASK;
>> +
>> +     status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>> +
>> +     if (!(status & possible_status)) {
>> +             dev_dbg(i2c_dev->dev,
>> +                     "spurious err it (status=0x%08x, ien=0x%08x)\n",
>> +                     status, ien);
>> +             return IRQ_NONE;
>> +     }
>> +
>> +     /* Use __fls() to check error bits first */
>> +     flag = __fls(status & possible_status);
>> +
>> +     switch (1 << flag) {
>> +     case STM32F4_I2C_SR1_BERR:
>> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
>> +             msg->result = -EIO;
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_ARLO:
>> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
>> +             msg->result = -EAGAIN;
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_AF:
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +             msg->result = -EIO;
>> +             break;
>> +
>> +     default:
>> +             dev_err(i2c_dev->dev,
>> +                     "err it unhandled: status=0x%08x)\n", status);
>> +             return IRQ_NONE;
>> +     }
>
> You only check a single irq flag here.
Yes only the first error could be reported to the i2c clients via
msg->result that's why I don't check all errors.
Moreover, as soon as an error occurs, the I2C device is reset.

>
>> +
>> +     stm32f4_i2c_soft_reset(i2c_dev);
>> +     stm32f4_i2c_disable_irq(i2c_dev);
>> +     complete(&i2c_dev->complete);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_xfer_msg() - Transfer a single I2C message
>> + * @i2c_dev: Controller's private data
>> + * @msg: I2C message to transfer
>> + * @is_first: first message of the sequence
>> + * @is_last: last message of the sequence
>> + */
>> +static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
>> +                             struct i2c_msg *msg, bool is_first,
>> +                             bool is_last)
>> +{
>> +     struct stm32f4_i2c_msg *f4_msg = &i2c_dev->msg;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +     unsigned long timeout;
>> +     u32 mask;
>> +     int ret;
>> +
>> +     f4_msg->addr = i2c_8bit_addr_from_msg(msg);
>> +     f4_msg->buf = msg->buf;
>> +     f4_msg->count = msg->len;
>> +     f4_msg->result = 0;
>> +     f4_msg->stop = is_last;
>> +
>> +     reinit_completion(&i2c_dev->complete);
>> +
>> +     /* Enable events and errors interrupts */
>> +     mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>> +     stm32f4_i2c_set_bits(i2c_dev->base + STM32F4_I2C_CR2, mask);
>> +
>> +     if (is_first) {
>> +             ret = stm32f4_i2c_wait_free_bus(i2c_dev);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             /* START generation */
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +     }
>> +
>> +     timeout = wait_for_completion_timeout(&i2c_dev->complete,
>> +                                           i2c_dev->adap.timeout);
>> +     ret = f4_msg->result;
>> +
>> +     /* Disable PEC position Ack */
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_POS);
>
> This is the only place mentioning PEC. Should this be about POS instead?
Yes you are right. Thanks

>
>> +
>> +     if (!timeout)
>> +             ret = -ETIMEDOUT;
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>> + * @i2c_adap: Adapter pointer to the controller
>> + * @msgs: Pointer to data to be written.
>> + * @num: Number of messages to be executed
>> + */
>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
>> +                         int num)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>> +     int ret, i;
>> +
>> +     ret = clk_enable(i2c_dev->clk);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "Failed to enable clock\n");
>> +             return ret;
>> +     }
>> +
>> +     stm32f4_i2c_hw_config(i2c_dev);
>> +
>> +     for (i = 0; i < num && !ret; i++)
>> +             ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
>> +                                        i == num - 1);
>> +
>> +     clk_disable(i2c_dev->clk);
>> +
>> +     return (ret < 0) ? ret : num;
>> +}
>> +
>> +static u32 stm32f4_i2c_func(struct i2c_adapter *adap)
>> +{
>> +     return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
>> +}
>> +
>> +static struct i2c_algorithm stm32f4_i2c_algo = {
>> +     .master_xfer = stm32f4_i2c_xfer,
>> +     .functionality = stm32f4_i2c_func,
>> +};
>> +
>> +static int stm32f4_i2c_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *np = pdev->dev.of_node;
>> +     struct stm32f4_i2c_dev *i2c_dev;
>> +     struct resource *res;
>> +     u32 irq_event, irq_error, clk_rate;
>> +     struct i2c_adapter *adap;
>> +     struct reset_control *rst;
>> +     int ret;
>> +
>> +     i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
>> +     if (!i2c_dev)
>> +             return -ENOMEM;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
>> +     if (IS_ERR(i2c_dev->base))
>> +             return PTR_ERR(i2c_dev->base);
>> +
>> +     irq_event = irq_of_parse_and_map(np, 0);
>> +     if (!irq_event) {
>> +             dev_err(&pdev->dev, "IRQ event missing or invalid\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     irq_error = irq_of_parse_and_map(np, 1);
>> +     if (!irq_error) {
>> +             dev_err(&pdev->dev, "IRQ error missing or invalid\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(i2c_dev->clk)) {
>> +             dev_err(&pdev->dev, "Error: Missing controller clock\n");
>> +             return PTR_ERR(i2c_dev->clk);
>> +     }
>> +     ret = clk_prepare(i2c_dev->clk);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "Failed to prepare clock\n");
>> +             return ret;
>> +     }
>> +
>> +     rst = devm_reset_control_get(&pdev->dev, NULL);
>> +     if (IS_ERR(rst)) {
>> +             dev_err(&pdev->dev, "Error: Missing controller reset\n");
>> +             ret = PTR_ERR(rst);
>> +             goto clk_free;
>> +     }
>> +     reset_control_assert(rst);
>> +     udelay(2);
>> +     reset_control_deassert(rst);
>> +
>> +     i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
>> +     ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
>> +     if (!ret && clk_rate >= 40000)
>> +             i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
>> +
>> +     i2c_dev->dev = &pdev->dev;
>> +
>> +     ret = devm_request_irq(&pdev->dev, irq_event, stm32f4_i2c_isr_event, 0,
>> +                            pdev->name, i2c_dev);
>
> Starting here this irq might trigger. Can this happen? If so,
> stm32f4_i2c_isr_event is called without the adapter being registered.
> Probably not an issue as the controller was just reset.
No irq could be triggered as after resett, the I2C is not enable as PE bit = 0.

>
>> +     if (ret) {
>> +             dev_err(&pdev->dev, "Failed to request irq event %i\n",
>> +                     irq_event);
>> +             goto clk_free;
>> +     }
>> +
>> +     ret = devm_request_irq(&pdev->dev, irq_error, stm32f4_i2c_isr_error, 0,
>> +                            pdev->name, i2c_dev);
>> +     if (ret) {
>> +             dev_err(&pdev->dev, "Failed to request irq error %i\n",
>> +                     irq_error);
>> +             goto clk_free;
>> +     }
>> +
>> +     adap = &i2c_dev->adap;
>> +     i2c_set_adapdata(adap, i2c_dev);
>> +     snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
>> +     adap->owner = THIS_MODULE;
>> +     adap->timeout = 2 * HZ;
>> +     adap->retries = 0;
>> +     adap->algo = &stm32f4_i2c_algo;
>> +     adap->dev.parent = &pdev->dev;
>> +     adap->dev.of_node = pdev->dev.of_node;
>> +
>> +     init_completion(&i2c_dev->complete);
>> +
>> +     ret = i2c_add_adapter(adap);
>> +     if (ret)
>> +             goto clk_free;
>> +
>> +     platform_set_drvdata(pdev, i2c_dev);
>> +
>> +     dev_info(i2c_dev->dev, "STM32F4 I2C driver registered\n");
>> +
>> +     return 0;
>> +
>> +clk_free:
>> +     clk_unprepare(i2c_dev->clk);
>> +     return ret;
>> +}
>> +
>> +static int stm32f4_i2c_remove(struct platform_device *pdev)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
>> +
>> +     i2c_del_adapter(&i2c_dev->adap);
>> +
>> +     clk_unprepare(i2c_dev->clk);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct of_device_id stm32f4_i2c_match[] = {
>> +     { .compatible = "st,stm32f4-i2c", },
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
>> +
>> +static struct platform_driver stm32f4_i2c_driver = {
>> +     .driver = {
>> +             .name = "stm32f4-i2c",
>> +             .of_match_table = stm32f4_i2c_match,
>> +     },
>> +     .probe = stm32f4_i2c_probe,
>> +     .remove = stm32f4_i2c_remove,
>> +};
>> +
>> +module_platform_driver(stm32f4_i2c_driver);
>> +
>> +MODULE_AUTHOR("M'boumba Cedric Madianga <cedric.madianga@gmail.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics STM32F4 I2C driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH 1/1] ARM64: dts: meson-gxbb-odroidc2: linux,usable-memory
From: Heinrich Schuchardt @ 2016-12-23 12:52 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Carlo Caione, Kevin Hilman, Neil Armstrong
  Cc: Heinrich Schuchardt, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic

After reading the fdt u-boot overwrites the reg property of the
memory node with <0x0 0x0 0x0 0x78000000>.

To override this setting we have to use the property
linux,usable-memory.

If the first 16MB of the 2GB physical memory are used by
the Linux kernel oops occur on the Odroid C2.

For the Odroid C2 this patch replaces
[RFT PATCH] ARM64: dts: meson-gxbb: Add reserved memory zone and
usable memory range
https://lkml.org/lkml/2016/12/12/127

Fixes: 855960342d1e7 ARM64: dts: amlogic: add Hardkernel ODROID-C2
CC: Neil Armstrong <narmstrong@baylibre.com>
CC: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbeacd330..82ab94417940 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -61,7 +61,7 @@
 
 	memory@0 {
 		device_type = "memory";
-		reg = <0x0 0x0 0x0 0x80000000>;
+		linux,usable-memory = <0x0 0x01000000 0x0 0x7f000000>;
 	};
 
 	usb_otg_pwr: regulator-usb-pwrs {
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] iio: misc: add a generic regulator driver
From: Geert Uytterhoeven @ 2016-12-23 12:56 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Bartosz Golaszewski, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-devicetree, LKML,
	Kevin Hilman, Patrick Titiano, Neil Armstrong, Liam Girdwood,
	Mark Brown
In-Reply-To: <9609b56b-194c-9899-1142-ff2ee285c6bd-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>

Hi Lars,

On Fri, Dec 23, 2016 at 12:35 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
> On 12/23/2016 11:00 AM, Geert Uytterhoeven wrote:
>> On Mon, Dec 12, 2016 at 6:15 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
>>> On 12/06/2016 12:12 PM, Bartosz Golaszewski wrote:
>>>> We're already using libiio to read the measured data from the power
>>>> monitor, that's why we'd like to use the iio framework for
>>>> power-cycling the devices as well. My question is: would bridging the
>>>> regulator framework be the right solution? Should we look for
>>>> something else? Bridge the GPIO framework instead?
>>>
>>> I wouldn't necessaries create bridge, but instead just use the GPIO
>>> framework directly.
>>>
>>> We now have the GPIO chardev interface which meant to be used to support
>>> application specific logic that control the GPIOs, but where you don't want
>>> to write a kernel driver.
>>>
>>> My idea was to add GPIOs and GPIO chips as high level object inside libiio
>>> that can be accessed through the same context as the IIO devices. Similar to
>>> the current IIO API you have a API for gpios that allows to enumerate the
>>> GPIO devices and their pins as well as modify the pin state.
>>
>> That would mean libiio has access to all GPIOs, allowing a remote person
>> to not only control through iiod the GPIOs for industrial control, but also the
>> GPIOs not intended for export, right?
>
> Well, it is a policy question. Who gets access to what. Right now it is all
> or nothing, a privileged application gets access to all devices/GPIOs, a
> unprivileged application gets access to nothing. Same for GPIOs as well as
> IIO devices.
>
> iiod at the moment does not have any access control at all, which in itself
> is a problem. We need to add support for that at some point. I don't see an
> issue with implementing a finer grained access scheme when we do so. E.g.
> unprivileged applications only get access to certain pins.

OK, so that's WIP.

>> Having a separate GPIO switch driver avoids that, as DT (or some other means)
>> can be used to specify and label the GPIOs for IIO use.
>
> Sure, functionally this would be equivalent, but we have to ask whether this
> is the right way to use the DT. Is access policy specification part of the
> hardware description? In my opinion the answer is no. At the hardware
> description level there is no operating system, there is no userspace or
> kernelspace, there is are no access levels. Putting the distinction between
> a switch/regulator that can be controlled from userspace or can only be
> controlled from kernel space into the DT would be a layering violation. It
> is analogous to why we don't have spidev DT bindings. This is an issue that
> needs to be solved at a higher level. In my opinion this level is a
> cooperation between kernel- and userspace. Kernelspace offering an interface
> to export a device for userspace access and userspace making use of that
> interface to request access to a device. In a similar way to how vfio is
> structured.

I'm not advocating using DT for policy, only for hardware description.

We have means (bindings) to describe GPIOs connected to LEDs and switches
(incl. their labels), while you can control LEDs through plain GPIO sysfs
export or chardev, too. It's just more error prone to use the latter.

We do not have bindings to describe GPIOs connected to e.g. relays.

Switching external devices (the internals of those devices not described
itself in DT, like in an industrial context), sounds more like something to
be handled by IIO, doesn't it?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 v7 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: M'boumba Cedric Madianga @ 2016-12-23 13:09 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: devicetree, Alexandre Torgue, Wolfram Sang, linux-kernel,
	Linus Walleij, Patrice Chotard, linux, Rob Herring, linux-i2c,
	Maxime Coquelin, linux-arm-kernel
In-Reply-To: <20161222191103.vzmfhnrtrzw2ivwa@pengutronix.de>

Hi,


2016-12-22 20:11 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
>
> On Thu, Dec 22, 2016 at 02:35:02PM +0100, M'boumba Cedric Madianga wrote:
>> @@ -337,6 +350,16 @@
>>                                       slew-rate = <2>;
>>                               };
>>                       };
>> +
>> +                     i2c1_pins_b: i2c1@0 {
>> +                             pins1 {
>> +                                     pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>;
>> +                                     drive-open-drain;
>> +                             };
>> +                             pins2 {
>> +                                     pinmux = <STM32F429_PB6_FUNC_I2C1_SCL>;
>> +                             };
>
> the second doesn't need the open-drain property? Why?
I thought that open-drain was only needed for SDA line.
But after double-checking I2C specification, it seems that SDA and SCL
lines need open-drain or open-collector to perform the wired-AND
function.
I will do some trials with this config and will fix it in the V8.
Thanks

>
>> +                     };
>>               };
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply

* [PATCH 1/2] ARM: dts: imx6dl: Add Engicam i.CoreM6 DualLite/Solo RQS initial support
From: Jagan Teki @ 2016-12-23 16:02 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-kernel, devicetree, linux-kernel, Matteo Lisi,
	Michael Trimarchi, Jagan Teki

From: Jagan Teki <jagan@amarulasolutions.com>

i.CoreM6 DualLite/Solo modules are system on module solutions manufactured
by Engicam with following characteristics:
CPU           NXP i.MX6 DL, 800MHz
RAM           1GB, 32, 64 bit, DDR3-800/1066
NAND          SLC,512MB
Power supply  Single 5V
MAX LCD RES   FULLHD

and more info at
http://www.engicam.com/en/products/embedded/som/standard/i-core-rqs-m6s-dl-d-q

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/boot/dts/Makefile             |  1 +
 arch/arm/boot/dts/imx6dl-icore-rqs.dts | 51 ++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6dl-icore-rqs.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index cccdbcb..51f8dae 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -349,6 +349,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
 	imx6dl-gw553x.dtb \
 	imx6dl-hummingboard.dtb \
 	imx6dl-icore.dtb \
+	imx6dl-icore-rqs.dtb \
 	imx6dl-nit6xlite.dtb \
 	imx6dl-nitrogen6x.dtb \
 	imx6dl-phytec-pbab01.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-icore-rqs.dts b/arch/arm/boot/dts/imx6dl-icore-rqs.dts
new file mode 100644
index 0000000..5c19927
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-icore-rqs.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2016 Amarula Solutions B.V.
+ * Copyright (C) 2016 Engicam S.r.l.
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License
+ *     version 2 as published by the Free Software Foundation.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "imx6q.dtsi"
+#include "imx6qdl-icore-rqs.dtsi"
+
+/ {
+	model = "Engicam i.CoreM6 DualLite/Solo RQS Starter Kit";
+	compatible = "engicam,imx6-icore-rqs", "fsl,imx6dl";
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: imx6q-icore-rqs: Update model to support Dual SOM
From: Jagan Teki @ 2016-12-23 16:02 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-kernel, devicetree, linux-kernel, Matteo Lisi,
	Michael Trimarchi, Jagan Teki
In-Reply-To: <1482508935-9414-1-git-send-email-jagan@openedev.com>

From: Jagan Teki <jagan@amarulasolutions.com>

Engicam i.CoreM6 Dual and Quad SOM's use same dts file, hence
update model name to add Dual and also added full mode decsription.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/boot/dts/imx6q-icore-rqs.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6q-icore-rqs.dts b/arch/arm/boot/dts/imx6q-icore-rqs.dts
index 0053188..76757f8 100644
--- a/arch/arm/boot/dts/imx6q-icore-rqs.dts
+++ b/arch/arm/boot/dts/imx6q-icore-rqs.dts
@@ -45,7 +45,7 @@
 #include "imx6qdl-icore-rqs.dtsi"
 
 / {
-	model = "Engicam i.CoreM6 Quad SOM";
+	model = "Engicam i.CoreM6 Quad/Dual RQS Starter Kit";
 	compatible = "engicam,imx6-icore-rqs", "fsl,imx6q";
 
 	sound {
-- 
1.9.1

^ permalink raw reply related

* [PATCH] Documentation: devicetree: "linux,usable-memory" property
From: Heinrich Schuchardt @ 2016-12-23 16:17 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Heinrich Schuchardt

Memory nodes may have a "linux,usable-memory" property
overriding the reg property.

This patch adds the missing documentation.

Signed-off-by: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
---
 Documentation/devicetree/booting-without-of.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt
index 280d283304bb..92712c5604b0 100644
--- a/Documentation/devicetree/booting-without-of.txt
+++ b/Documentation/devicetree/booting-without-of.txt
@@ -981,6 +981,10 @@ compatibility.
       removed later. The kernel can take this into consideration when
       doing nonmovable allocations and when laying out memory zones.
 
+    - linux,usable-memory : This property overrides the reg property.
+      It can be used if the bootloader changes the reg property to
+      improper values.
+
   e) The /chosen node
 
   This node is a bit "special". Normally, that's where Open Firmware
-- 
2.11.0

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

^ permalink raw reply related

* [PATCH 1/2] input: touchscreen: add driver for Zeitec ZET6223
From: Jelle van der Waa @ 2016-12-23 16:32 UTC (permalink / raw)
  To: Rob Herring, Dmitry Torokhov, linux-input, devicetree; +Cc: Jelle van der Waa

This is a basic driver for the Zeitec ZET6223 I2C touchscreen
controllers. The driver does not support firmware loading, which is not
required for all tablets which contain this chip.

Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
---
 .../bindings/input/touchscreen/zet6223.txt         |  29 +++
 drivers/input/touchscreen/Kconfig                  |  11 ++
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/zet6223.c                | 198 +++++++++++++++++++++
 4 files changed, 239 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
 create mode 100644 drivers/input/touchscreen/zet6223.c

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt b/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
new file mode 100644
index 000000000000..cc0f7d06bc8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
@@ -0,0 +1,29 @@
+Zeitec ZET6223 I2C touchscreen controller
+
+Required properties:
+ - compatible            : "zeitec,zet6223"
+ - reg                   : I2C slave address of the chip (0x76)
+ - interrupt-parent      : a phandle pointing to the interrupt controller
+                           serving the interrupt for this chip
+ - interrupts            : interrupt specification for the zet6223 interrupt
+
+Optional properties:
+
+- touchscreen-size-x     : See touchscreen.txt
+- touchscreen-size-y     : See touchscreen.txt
+- touchscreen-inverted-x  : See touchscreen.txt
+- touchscreen-inverted-y  : See touchscreen.txt
+- touchscreen-swapped-x-y : See touchscreen.txt
+
+Example:
+
+i2c@00000000 {
+
+       zet6223: touchscreen@76 {
+               compatible = "zeitec,zet6223";
+               reg = <0x76>;
+               interrupt-parent = <&pio>;
+               interrupts = <6 11 IRQ_TYPE_EDGE_FALLING>
+       };
+
+};
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index efca0133e266..3c70adfe676d 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1214,4 +1214,15 @@ config TOUCHSCREEN_ROHM_BU21023
 	  To compile this driver as a module, choose M here: the
 	  module will be called bu21023_ts.
 
+config TOUCHSCREEN_ZET6223
+       tristate "Zeitec ZET6223 touchscreen driver"
+       depends on I2C
+       help
+         Say Y here if you have a touchscreen using Zeitec ZET6223
+
+         If unsure, say N.
+
+         To compile this driver as a module, choose M here: the
+         module will be called zet6223.
+
 endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 81b86451782d..11f8953613cd 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_TOUCHSCREEN_TPS6507X)	+= tps6507x-ts.o
 obj-$(CONFIG_TOUCHSCREEN_ZFORCE)	+= zforce_ts.o
 obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50)	+= colibri-vf50-ts.o
 obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023)	+= rohm_bu21023.o
+obj-$(CONFIG_TOUCHSCREEN_ZET6223)       += zet6223.o
diff --git a/drivers/input/touchscreen/zet6223.c b/drivers/input/touchscreen/zet6223.c
new file mode 100644
index 000000000000..aecae06877cf
--- /dev/null
+++ b/drivers/input/touchscreen/zet6223.c
@@ -0,0 +1,198 @@
+/*
+ * Copyright (C) 2016, Jelle van der Waa <jelle@vdwaa.nl>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License as published by the Free
+ *  Software Foundation; either version 2 of the License, or (at your option)
+ *  any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ *  more details.
+ */
+
+#include <asm/unaligned.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/module.h>
+
+#define ZET6223_CMD_INFO 0xB2
+#define ZET6223_CMD_INFO_LENGTH 17
+#define ZET6223_VALID_PACKET 0x3c
+
+struct zet6223_data {
+	struct i2c_client *client;
+	struct input_dev *input;
+	struct touchscreen_properties prop;
+	u8 fingernum;
+};
+
+static int zet6223_start(struct input_dev *dev)
+{
+	struct zet6223_data *data = input_get_drvdata(dev);
+
+	enable_irq(data->client->irq);
+
+	return 0;
+}
+
+static void zet6223_stop(struct input_dev *dev)
+{
+	struct zet6223_data *data = input_get_drvdata(dev);
+
+	disable_irq(data->client->irq);
+}
+
+static irqreturn_t irqreturn_t_zet6223(int irq, void *dev_id)
+{
+	struct zet6223_data *data = dev_id;
+	struct device *dev = &data->client->dev;
+	/*
+	 * First 3 bytes are an identifier, two bytes of finger data.
+	 * X, Y data per finger is 4 bytes.
+	 */
+	u8 bufsize = 3 + 4 * data->fingernum;
+	u8 buf[bufsize];
+	u8 i;
+	u16 finger_bits;
+	int ret;
+
+	ret = i2c_master_recv(data->client, buf, bufsize);
+	if (ret != bufsize) {
+		dev_err_ratelimited(dev, "Error reading input data: %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	if (buf[0] != ZET6223_VALID_PACKET)
+		return IRQ_HANDLED;
+
+	finger_bits = get_unaligned_be16(buf + 1);
+	for (i = 0; i < data->fingernum; i++) {
+		if (!(finger_bits & BIT(15 - i)))
+			continue;
+
+		input_mt_slot(data->input, i);
+		input_mt_report_slot_state(data->input, MT_TOOL_FINGER, true);
+		input_event(data->input, EV_ABS, ABS_MT_POSITION_X,
+				((buf[i + 3] >> 4) << 8) + buf[i + 4]);
+		input_event(data->input, EV_ABS, ABS_MT_POSITION_Y,
+				((buf[i + 3] & 0xF) << 8) + buf[i + 5]);
+	}
+
+	input_mt_sync_frame(data->input);
+	input_sync(data->input);
+
+	return IRQ_HANDLED;
+}
+
+static int zet6223_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct zet6223_data *data;
+	struct input_dev *input;
+	u8 buf[ZET6223_CMD_INFO_LENGTH];
+	u8 cmd = ZET6223_CMD_INFO;
+	int ret;
+
+	if (!client->irq) {
+		dev_err(dev, "Error no irq specified\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = i2c_master_send(client, &cmd, 1);
+	if (ret < 0) {
+		dev_err(dev, "touchpanel info cmd failed: %d\n", ret);
+		return -ENODEV;
+	}
+
+	ret = i2c_master_recv(client, buf, ZET6223_CMD_INFO_LENGTH);
+	if (ret < 0) {
+		dev_err(dev, "cannot retrieve touchpanel info: %d\n", ret);
+		return -ENODEV;
+	}
+
+	data->fingernum = buf[15] & 0x7F;
+	if (data->fingernum > 16) {
+		data->fingernum = 16;
+		dev_warn(dev, "touchpanel reports more then 16 fingers, limit to 16");
+	}
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	input_set_abs_params(input, ABS_MT_POSITION_X, 0,
+			get_unaligned_le16(&buf[8]), 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
+			get_unaligned_le16(&buf[10]), 0, 0);
+	touchscreen_parse_properties(input, true, &data->prop);
+
+	input->name = client->name;
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = dev;
+	input->open = zet6223_start;
+	input->close = zet6223_stop;
+
+	ret = input_mt_init_slots(input, data->fingernum,
+		INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+	if (ret)
+		return ret;
+
+	data->client = client;
+	data->input = input;
+
+	input_set_drvdata(input, data);
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+			irqreturn_t_zet6223, IRQF_ONESHOT, client->name, data);
+	if (ret) {
+		dev_err(dev, "Error requesting irq: %d\n", ret);
+		return ret;
+	}
+
+	zet6223_stop(input);
+
+	ret = input_register_device(input);
+	if (ret)
+		return ret;
+
+	i2c_set_clientdata(client, data);
+
+	return 0;
+}
+
+static const struct of_device_id zet6223_of_match[] = {
+	{ .compatible = "zeitec", "zet6223" },
+	{ }
+};
+
+static const struct i2c_device_id zet6223_id[] = {
+	{ "zet6223", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, zet6223_id);
+
+static struct i2c_driver zet6223_driver = {
+	.driver = {
+		.name = "zet6223",
+		.of_match_table = zet6223_of_match,
+	},
+	.probe = zet6223_probe,
+	.id_table = zet6223_id
+};
+
+module_i2c_driver(zet6223_driver);
+
+MODULE_AUTHOR("Jelle van der Waa <jelle@vdwaa.nl>");
+MODULE_DESCRIPTION("ZEITEC zet622x I2C touchscreen driver");
+MODULE_LICENSE("GPL");
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/2] devicetree: add vendor prefix for Zeitec
From: Jelle van der Waa @ 2016-12-23 16:32 UTC (permalink / raw)
  To: Rob Herring, Dmitry Torokhov, linux-input, devicetree; +Cc: Jelle van der Waa
In-Reply-To: <20161223163214.7716-1-jelle@vdwaa.nl>

Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..a3d7608a5cb3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -329,6 +329,7 @@ xes	Extreme Engineering Solutions (X-ES)
 xillybus	Xillybus Ltd.
 xlnx	Xilinx
 zarlink	Zarlink Semiconductor
+zeitec  ZEITEC Semiconductor Co., LTD.
 zii	Zodiac Inflight Innovations
 zte	ZTE Corp.
 zyxel	ZyXEL Communications Corp.
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH v4 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Marek Vasut @ 2016-12-23 19:00 UTC (permalink / raw)
  To: Shawn Lin, David Woodhouse, Brian Norris
  Cc: Cyrille Pitchen, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner
In-Reply-To: <1481794068-241619-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On 12/15/2016 10:27 AM, Shawn Lin wrote:
> Add rockchip serial flash controller driver
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Acked-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks !

-- 
Best regards,
Marek Vasut
--
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 v4 1/2] mtd: spi-nor: Bindings for Rockchip serial flash controller
From: Marek Vasut @ 2016-12-23 19:00 UTC (permalink / raw)
  To: Shawn Lin, David Woodhouse, Brian Norris
  Cc: Cyrille Pitchen, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner
In-Reply-To: <1481794068-241619-2-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On 12/15/2016 10:27 AM, Shawn Lin wrote:
> Add binding document for the Rockchip serial flash controller.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---

Acked-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

-- 
Best regards,
Marek Vasut
--
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/1] ARM64: dts: meson-gxbb-odroidc2: linux,usable-memory
From: Neil Armstrong @ 2016-12-23 19:48 UTC (permalink / raw)
  To: Heinrich Schuchardt, Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, Carlo Caione, Kevin Hilman
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161223125213.5461-1-xypron.glpk-Mmb7MZpHnFY@public.gmane.org>

Le 23/12/2016 13:52, Heinrich Schuchardt a écrit :
> After reading the fdt u-boot overwrites the reg property of the
> memory node with <0x0 0x0 0x0 0x78000000>.
> 
> To override this setting we have to use the property
> linux,usable-memory.
> 
> If the first 16MB of the 2GB physical memory are used by
> the Linux kernel oops occur on the Odroid C2.
> 
> For the Odroid C2 this patch replaces
> [RFT PATCH] ARM64: dts: meson-gxbb: Add reserved memory zone and
> usable memory range
> https://lkml.org/lkml/2016/12/12/127
> 
> Fixes: 855960342d1e7 ARM64: dts: amlogic: add Hardkernel ODROID-C2
> CC: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> CC: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index 238fbeacd330..82ab94417940 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -61,7 +61,7 @@
>  
>  	memory@0 {
>  		device_type = "memory";
> -		reg = <0x0 0x0 0x0 0x80000000>;
> +		linux,usable-memory = <0x0 0x01000000 0x0 0x7f000000>;
>  	};
>  
>  	usb_otg_pwr: regulator-usb-pwrs {
> 

Hi Heinrich,

Thanks a lot for testing and pushing this patch, indeed I missed this point abou u-boot overwriting reg.

I will test this next week, and hopefully get these merged.
Neil
--
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: [RFC PATCHv2 2/4] clk: mdm9615: Add EBI2 clock
From: Neil Armstrong @ 2016-12-23 19:50 UTC (permalink / raw)
  To: Zoran Markovic, linux-kernel
  Cc: Andy Gross, David Brown, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, linux-arm-msm, linux-soc, linux-clk,
	devicetree
In-Reply-To: <1482468884-31027-1-git-send-email-zmarkovic@sierrawireless.com>

Le 23/12/2016 05:54, Zoran Markovic a écrit :
> Add definition of EBI2 clock used by MDM9615 NAND controller.
> 
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: David Brown <david.brown@linaro.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-soc@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Zoran Markovic <zmarkovic@sierrawireless.com>
> ---
>  drivers/clk/qcom/gcc-mdm9615.c               |   30 ++++++++++++++++++++++++++
>  include/dt-bindings/clock/qcom,gcc-mdm9615.h |    3 +++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-mdm9615.c b/drivers/clk/qcom/gcc-mdm9615.c
> index 581a17f..e9e98b1 100644
> --- a/drivers/clk/qcom/gcc-mdm9615.c
> +++ b/drivers/clk/qcom/gcc-mdm9615.c
> @@ -1563,6 +1563,34 @@ enum {
>  	},
>  };
>  
> +static struct clk_branch ebi2_clk = {
> +	.hwcg_reg = 0x2664,
> +	.hwcg_bit = 6,
> +	.halt_reg = 0x2fcc,
> +	.halt_bit = 23,
> +	.clkr = {
> +		.enable_reg = 0x2664,
> +		.enable_mask = BIT(6) | BIT(4),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "ebi2_clk",
> +			.ops = &clk_branch_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_branch ebi2_aon_clk = {
> +	.halt_reg = 0x2fcc,
> +	.halt_bit = 23,
> +	.clkr = {
> +		.enable_reg = 0x2664,
> +		.enable_mask = BIT(8),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "ebi2_aon_clk",
> +			.ops = &clk_branch_ops,
> +		},
> +	},
> +};
> +
>  static struct clk_hw *gcc_mdm9615_hws[] = {
>  	&cxo.hw,
>  };
> @@ -1637,6 +1665,8 @@ enum {
>  	[PMIC_ARB1_H_CLK] = &pmic_arb1_h_clk.clkr,
>  	[PMIC_SSBI2_CLK] = &pmic_ssbi2_clk.clkr,
>  	[RPM_MSG_RAM_H_CLK] = &rpm_msg_ram_h_clk.clkr,
> +	[EBI2_CLK] = &ebi2_clk.clkr,
> +	[EBI2_AON_CLK] = &ebi2_aon_clk.clkr,
>  };
>  
>  static const struct qcom_reset_map gcc_mdm9615_resets[] = {
> diff --git a/include/dt-bindings/clock/qcom,gcc-mdm9615.h b/include/dt-bindings/clock/qcom,gcc-mdm9615.h
> index 9ab2c40..57cdca6 100644
> --- a/include/dt-bindings/clock/qcom,gcc-mdm9615.h
> +++ b/include/dt-bindings/clock/qcom,gcc-mdm9615.h
> @@ -323,5 +323,8 @@
>  #define CE3_H_CLK				305
>  #define USB_HS1_SYSTEM_CLK_SRC			306
>  #define USB_HS1_SYSTEM_CLK			307
> +#define EBI2_CLK				308
> +#define EBI2_AON_CLK				309
> +
>  
>  #endif
> 

Hi Zoran,

Thanks for this patch, we did not found the definition for these clocks at all !

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

^ permalink raw reply

* [RFC PATCH v2 0/4] hwmon: adc128d818: Support missing operation modes
From: Alexander Koch @ 2016-12-23 22:12 UTC (permalink / raw)
  To: linux-kernel, linux-hwmon, devicetree
  Cc: Rob Herring, Mark Rutland, Jean Delvare, Guenter Roeck,
	Jiri Kosina, Alexander Koch

The ADC128D818 offers four different chip operation modes which vary in the
number and measurement types of the available input signals (see datasheet
sec. 8.4.1).

The current version of the driver only supports the default chip operation
mode (mode 0), providing seven analog values and a temperature reading.

This patch series adds support for operation modes 1-3, selectable through
the device tree attribute 'mode':

        adc1: adc128d818@1d {
                compatible = "ti,adc128d818";
                reg = <0x1d>;
                mode = /bits/ 8 <1>;
        };

The changes are transparent as 'mode' defaults to mode 0 which yields the
previous behaviour.


Changes from v1:
 - Add bindings document as first patch
 - Preserve logical atomicity of code changes
 - Improve sysfs device node handling (use is_visible() instead of
   duplicate attribute list)
 - Add trivial code refactoring stage for checkpatch.pl to succeed


Alexander Koch (4):
  devicetree: hwmon: Add bindings for ADC128D818
  hwmon: adc128d818: Implement mode selection via dt
  hwmon: adc128d818: Trivial code style fixup
  hwmon: adc128d818: Support operation modes 1-3

 .../devicetree/bindings/hwmon/adc128d818.txt       |  39 ++++
 drivers/hwmon/adc128d818.c                         | 237 ++++++++++++---------
 2 files changed, 179 insertions(+), 97 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adc128d818.txt

-- 
2.11.0


^ permalink raw reply

* [RFC PATCH v2 1/4] devicetree: hwmon: Add bindings for ADC128D818
From: Alexander Koch @ 2016-12-23 22:12 UTC (permalink / raw)
  To: linux-kernel, linux-hwmon, devicetree
  Cc: Rob Herring, Mark Rutland, Jean Delvare, Guenter Roeck,
	Jiri Kosina, Alexander Koch
In-Reply-To: <20161223221205.8825-1-mail@alexanderkoch.net>

Add bindings documentation for the ADC128D818 driver, featuring default I2C
properties along with the optional 'mode' property for chip operation mode
selection (see datasheet, sec. 8.4.1).

Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
---
 .../devicetree/bindings/hwmon/adc128d818.txt       | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/adc128d818.txt

diff --git a/Documentation/devicetree/bindings/hwmon/adc128d818.txt b/Documentation/devicetree/bindings/hwmon/adc128d818.txt
new file mode 100644
index 000000000000..5e14aa36a696
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/adc128d818.txt
@@ -0,0 +1,39 @@
+TI ADC128D818 ADC System Monitor With Temperature Sensor
+--------------------------------------------------------
+
+Operation modes:
+
+ - Mode 0:  7 single-ended voltage readings (IN0-IN6),
+            1 temperature reading (internal)
+ - Mode 1:  8 single-ended voltage readings (IN0-IN7),
+            no temperature
+ - Mode 2:  4 pseudo-differential voltage readings
+              (IN0-IN1, IN3-IN2, IN4-IN5, IN7-IN6),
+            1 temperature reading (internal)
+ - Mode 3:  4 single-ended voltage readings (IN0-IN3),
+            2 pseudo-differential voltage readings
+              (IN4-IN5, IN7-IN6),
+            1 temperature reading (internal)
+
+If no operation mode is configured via device tree, the driver defaults
+to Mode 0.
+
+
+Required node properties:
+
+ - compatible:  must be set to "ti,adc128d818"
+ - reg:         I2C address of the device
+
+Optional node properties:
+
+ - mode:        Operation mode (see above).
+
+
+Example (operation mode 2):
+
+	adc128d818@1d {
+		compatible = "ti,adc128d818";
+		reg = <0x1d>;
+		mode = /bits/ 8 <2>;
+	};
+
-- 
2.11.0


^ permalink raw reply related

* [RFC PATCH v2 2/4] hwmon: adc128d818: Implement mode selection via dt
From: Alexander Koch @ 2016-12-23 22:12 UTC (permalink / raw)
  To: linux-kernel, linux-hwmon, devicetree
  Cc: Rob Herring, Mark Rutland, Jean Delvare, Guenter Roeck,
	Jiri Kosina, Alexander Koch
In-Reply-To: <20161223221205.8825-1-mail@alexanderkoch.net>

Implement operation mode selection using the optional 'mode' devicetree
property (see [1]). The ADC128D818 supports four operation modes differing
in the number and type of input readings (see datasheet, sec. 8.4.1), of
which mode 0 is the default.

We only add handling of the 'mode' property here, the driver still supports
nothing else than the default mode 0.

[1] Documentation/devicetree/bindings/hwmon/adc128d818.txt

Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
---
 drivers/hwmon/adc128d818.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c
index ad2b47e40345..8667f454ea11 100644
--- a/drivers/hwmon/adc128d818.c
+++ b/drivers/hwmon/adc128d818.c
@@ -28,6 +28,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/mutex.h>
 #include <linux/bitops.h>
+#include <linux/of.h>
 
 /* Addresses to scan
  * The chip also supports addresses 0x35..0x37. Don't scan those addresses
@@ -63,6 +64,7 @@ struct adc128_data {
 	struct regulator *regulator;
 	int vref;		/* Reference voltage in mV */
 	struct mutex update_lock;
+	u8 mode;		/* Operation mode */
 	bool valid;		/* true if following fields are valid */
 	unsigned long last_updated;	/* In jiffies */
 
@@ -387,6 +389,15 @@ static int adc128_init_client(struct adc128_data *data)
 	if (err)
 		return err;
 
+	/* Set operation mode, if non-default */
+	if (data->mode != 0) {
+		err = i2c_smbus_write_byte_data(client,
+						ADC128_REG_CONFIG_ADV,
+						data->mode << 1);
+		if (err)
+			return err;
+	}
+
 	/* Start monitoring */
 	err = i2c_smbus_write_byte_data(client, ADC128_REG_CONFIG, 0x01);
 	if (err)
@@ -433,6 +444,19 @@ static int adc128_probe(struct i2c_client *client,
 		data->vref = 2560;	/* 2.56V, in mV */
 	}
 
+	/* Operation mode is optional and defaults to mode 0 */
+	if (of_property_read_u8(dev->of_node, "mode", &data->mode) == 0) {
+		/* Currently only mode 0 supported */
+		if (data->mode != 0) {
+			dev_err(dev, "unsupported operation mode %d",
+				data->mode);
+			err = -EINVAL;
+			goto error;
+		}
+	} else {
+		data->mode = 0;
+	}
+
 	data->client = client;
 	i2c_set_clientdata(client, data);
 	mutex_init(&data->update_lock);
-- 
2.11.0


^ permalink raw reply related

* [RFC PATCH v2 3/4] hwmon: adc128d818: Trivial code style fixup
From: Alexander Koch @ 2016-12-23 22:12 UTC (permalink / raw)
  To: linux-kernel, linux-hwmon, devicetree
  Cc: Rob Herring, Mark Rutland, Jean Delvare, Guenter Roeck,
	Jiri Kosina, Alexander Koch
In-Reply-To: <20161223221205.8825-1-mail@alexanderkoch.net>

Replace sysfs symbolic file permissions, e.g. 'S_IRUGO', by octal
permissions. This fixes checkpatch.pl warnings.

Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
---
 drivers/hwmon/adc128d818.c | 99 ++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 60 deletions(-)

diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c
index 8667f454ea11..cbb3bc5e5229 100644
--- a/drivers/hwmon/adc128d818.c
+++ b/drivers/hwmon/adc128d818.c
@@ -242,69 +242,48 @@ static ssize_t adc128_show_alarm(struct device *dev,
 	return sprintf(buf, "%u\n", !!(alarms & mask));
 }
 
-static SENSOR_DEVICE_ATTR_2(in0_input, S_IRUGO,
-			    adc128_show_in, NULL, 0, 0);
-static SENSOR_DEVICE_ATTR_2(in0_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 0, 1);
-static SENSOR_DEVICE_ATTR_2(in0_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 0, 2);
-
-static SENSOR_DEVICE_ATTR_2(in1_input, S_IRUGO,
-			    adc128_show_in, NULL, 1, 0);
-static SENSOR_DEVICE_ATTR_2(in1_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 1, 1);
-static SENSOR_DEVICE_ATTR_2(in1_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 1, 2);
-
-static SENSOR_DEVICE_ATTR_2(in2_input, S_IRUGO,
-			    adc128_show_in, NULL, 2, 0);
-static SENSOR_DEVICE_ATTR_2(in2_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 2, 1);
-static SENSOR_DEVICE_ATTR_2(in2_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 2, 2);
-
-static SENSOR_DEVICE_ATTR_2(in3_input, S_IRUGO,
-			    adc128_show_in, NULL, 3, 0);
-static SENSOR_DEVICE_ATTR_2(in3_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 3, 1);
-static SENSOR_DEVICE_ATTR_2(in3_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 3, 2);
-
-static SENSOR_DEVICE_ATTR_2(in4_input, S_IRUGO,
-			    adc128_show_in, NULL, 4, 0);
-static SENSOR_DEVICE_ATTR_2(in4_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 4, 1);
-static SENSOR_DEVICE_ATTR_2(in4_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 4, 2);
-
-static SENSOR_DEVICE_ATTR_2(in5_input, S_IRUGO,
-			    adc128_show_in, NULL, 5, 0);
-static SENSOR_DEVICE_ATTR_2(in5_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 5, 1);
-static SENSOR_DEVICE_ATTR_2(in5_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 5, 2);
-
-static SENSOR_DEVICE_ATTR_2(in6_input, S_IRUGO,
-			    adc128_show_in, NULL, 6, 0);
-static SENSOR_DEVICE_ATTR_2(in6_min, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 6, 1);
-static SENSOR_DEVICE_ATTR_2(in6_max, S_IWUSR | S_IRUGO,
-			    adc128_show_in, adc128_set_in, 6, 2);
-
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, adc128_show_temp, NULL, 0);
-static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
+static SENSOR_DEVICE_ATTR_2(in0_input, 0444, adc128_show_in, NULL, 0, 0);
+static SENSOR_DEVICE_ATTR_2(in0_min, 0644, adc128_show_in, adc128_set_in, 0, 1);
+static SENSOR_DEVICE_ATTR_2(in0_max, 0644, adc128_show_in, adc128_set_in, 0, 2);
+
+static SENSOR_DEVICE_ATTR_2(in1_input, 0444, adc128_show_in, NULL, 1, 0);
+static SENSOR_DEVICE_ATTR_2(in1_min, 0644, adc128_show_in, adc128_set_in, 1, 1);
+static SENSOR_DEVICE_ATTR_2(in1_max, 0644, adc128_show_in, adc128_set_in, 1, 2);
+
+static SENSOR_DEVICE_ATTR_2(in2_input, 0444, adc128_show_in, NULL, 2, 0);
+static SENSOR_DEVICE_ATTR_2(in2_min, 0644, adc128_show_in, adc128_set_in, 2, 1);
+static SENSOR_DEVICE_ATTR_2(in2_max, 0644, adc128_show_in, adc128_set_in, 2, 2);
+
+static SENSOR_DEVICE_ATTR_2(in3_input, 0444, adc128_show_in, NULL, 3, 0);
+static SENSOR_DEVICE_ATTR_2(in3_min, 0644, adc128_show_in, adc128_set_in, 3, 1);
+static SENSOR_DEVICE_ATTR_2(in3_max, 0644, adc128_show_in, adc128_set_in, 3, 2);
+
+static SENSOR_DEVICE_ATTR_2(in4_input, 0444, adc128_show_in, NULL, 4, 0);
+static SENSOR_DEVICE_ATTR_2(in4_min, 0644, adc128_show_in, adc128_set_in, 4, 1);
+static SENSOR_DEVICE_ATTR_2(in4_max, 0644, adc128_show_in, adc128_set_in, 4, 2);
+
+static SENSOR_DEVICE_ATTR_2(in5_input, 0444, adc128_show_in, NULL, 5, 0);
+static SENSOR_DEVICE_ATTR_2(in5_min, 0644, adc128_show_in, adc128_set_in, 5, 1);
+static SENSOR_DEVICE_ATTR_2(in5_max, 0644, adc128_show_in, adc128_set_in, 5, 2);
+
+static SENSOR_DEVICE_ATTR_2(in6_input, 0444, adc128_show_in, NULL, 6, 0);
+static SENSOR_DEVICE_ATTR_2(in6_min, 0644, adc128_show_in, adc128_set_in, 6, 1);
+static SENSOR_DEVICE_ATTR_2(in6_max, 0644, adc128_show_in, adc128_set_in, 6, 2);
+
+static SENSOR_DEVICE_ATTR(temp1_input, 0444, adc128_show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, 0644,
 			  adc128_show_temp, adc128_set_temp, 1);
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp1_max_hyst, 0644,
 			  adc128_show_temp, adc128_set_temp, 2);
 
-static SENSOR_DEVICE_ATTR(in0_alarm, S_IRUGO, adc128_show_alarm, NULL, 0);
-static SENSOR_DEVICE_ATTR(in1_alarm, S_IRUGO, adc128_show_alarm, NULL, 1);
-static SENSOR_DEVICE_ATTR(in2_alarm, S_IRUGO, adc128_show_alarm, NULL, 2);
-static SENSOR_DEVICE_ATTR(in3_alarm, S_IRUGO, adc128_show_alarm, NULL, 3);
-static SENSOR_DEVICE_ATTR(in4_alarm, S_IRUGO, adc128_show_alarm, NULL, 4);
-static SENSOR_DEVICE_ATTR(in5_alarm, S_IRUGO, adc128_show_alarm, NULL, 5);
-static SENSOR_DEVICE_ATTR(in6_alarm, S_IRUGO, adc128_show_alarm, NULL, 6);
-static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adc128_show_alarm, NULL, 7);
+static SENSOR_DEVICE_ATTR(in0_alarm, 0444, adc128_show_alarm, NULL, 0);
+static SENSOR_DEVICE_ATTR(in1_alarm, 0444, adc128_show_alarm, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_alarm, 0444, adc128_show_alarm, NULL, 2);
+static SENSOR_DEVICE_ATTR(in3_alarm, 0444, adc128_show_alarm, NULL, 3);
+static SENSOR_DEVICE_ATTR(in4_alarm, 0444, adc128_show_alarm, NULL, 4);
+static SENSOR_DEVICE_ATTR(in5_alarm, 0444, adc128_show_alarm, NULL, 5);
+static SENSOR_DEVICE_ATTR(in6_alarm, 0444, adc128_show_alarm, NULL, 6);
+static SENSOR_DEVICE_ATTR(temp1_max_alarm, 0444, adc128_show_alarm, NULL, 7);
 
 static struct attribute *adc128_attrs[] = {
 	&sensor_dev_attr_in0_min.dev_attr.attr,
-- 
2.11.0


^ permalink raw reply related

* [RFC PATCH v2 4/4] hwmon: adc128d818: Support operation modes 1-3
From: Alexander Koch @ 2016-12-23 22:12 UTC (permalink / raw)
  To: linux-kernel, linux-hwmon, devicetree
  Cc: Rob Herring, Mark Rutland, Jean Delvare, Guenter Roeck,
	Jiri Kosina, Alexander Koch
In-Reply-To: <20161223221205.8825-1-mail@alexanderkoch.net>

Add support for operation modes 1-3 of the ADC128D818 (see datasheet sec.
8.4.1). These differ in the number and type of the available input signals,
requiring the driver to selectively hide sysfs nodes according to the
operation mode configured via devicetree.

Signed-off-by: Alexander Koch <mail@alexanderkoch.net>
---
 drivers/hwmon/adc128d818.c | 120 ++++++++++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 40 deletions(-)

diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c
index cbb3bc5e5229..46c9fc9f1be2 100644
--- a/drivers/hwmon/adc128d818.c
+++ b/drivers/hwmon/adc128d818.c
@@ -59,6 +59,9 @@ static const unsigned short normal_i2c[] = {
 #define ADC128_REG_MAN_ID		0x3e
 #define ADC128_REG_DEV_ID		0x3f
 
+/* Voltage inputs visible per operation mode */
+static const u8 num_inputs[] = { 7, 8, 4, 6 };
+
 struct adc128_data {
 	struct i2c_client *client;
 	struct regulator *regulator;
@@ -68,7 +71,7 @@ struct adc128_data {
 	bool valid;		/* true if following fields are valid */
 	unsigned long last_updated;	/* In jiffies */
 
-	u16 in[3][7];		/* Register value, normalized to 12 bit
+	u16 in[3][8];		/* Register value, normalized to 12 bit
 				 * 0: input voltage
 				 * 1: min limit
 				 * 2: max limit
@@ -89,7 +92,7 @@ static struct adc128_data *adc128_update_device(struct device *dev)
 	mutex_lock(&data->update_lock);
 
 	if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
-		for (i = 0; i < 7; i++) {
+		for (i = 0; i < num_inputs[data->mode]; i++) {
 			rv = i2c_smbus_read_word_swapped(client,
 							 ADC128_REG_IN(i));
 			if (rv < 0)
@@ -109,20 +112,25 @@ static struct adc128_data *adc128_update_device(struct device *dev)
 			data->in[2][i] = rv << 4;
 		}
 
-		rv = i2c_smbus_read_word_swapped(client, ADC128_REG_TEMP);
-		if (rv < 0)
-			goto abort;
-		data->temp[0] = rv >> 7;
+		if (data->mode != 1) {
+			rv = i2c_smbus_read_word_swapped(client,
+							 ADC128_REG_TEMP);
+			if (rv < 0)
+				goto abort;
+			data->temp[0] = rv >> 7;
 
-		rv = i2c_smbus_read_byte_data(client, ADC128_REG_TEMP_MAX);
-		if (rv < 0)
-			goto abort;
-		data->temp[1] = rv << 1;
+			rv = i2c_smbus_read_byte_data(client,
+						      ADC128_REG_TEMP_MAX);
+			if (rv < 0)
+				goto abort;
+			data->temp[1] = rv << 1;
 
-		rv = i2c_smbus_read_byte_data(client, ADC128_REG_TEMP_HYST);
-		if (rv < 0)
-			goto abort;
-		data->temp[2] = rv << 1;
+			rv = i2c_smbus_read_byte_data(client,
+						      ADC128_REG_TEMP_HYST);
+			if (rv < 0)
+				goto abort;
+			data->temp[2] = rv << 1;
+		}
 
 		rv = i2c_smbus_read_byte_data(client, ADC128_REG_ALARM);
 		if (rv < 0)
@@ -242,6 +250,25 @@ static ssize_t adc128_show_alarm(struct device *dev,
 	return sprintf(buf, "%u\n", !!(alarms & mask));
 }
 
+static umode_t adc128_is_visible(struct kobject *kobj,
+				 struct attribute *attr, int index)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct adc128_data *data = dev_get_drvdata(dev);
+
+	if (index < 8 * 4) {
+		/* Voltage, visible according to num_inputs[] */
+		if (index >= num_inputs[data->mode] * 4)
+			return 0;
+	} else {
+		/* Temperature, visible if not in mode 1 */
+		if (data->mode == 1)
+			return 0;
+	}
+
+	return attr->mode;
+}
+
 static SENSOR_DEVICE_ATTR_2(in0_input, 0444, adc128_show_in, NULL, 0, 0);
 static SENSOR_DEVICE_ATTR_2(in0_min, 0644, adc128_show_in, adc128_set_in, 0, 1);
 static SENSOR_DEVICE_ATTR_2(in0_max, 0644, adc128_show_in, adc128_set_in, 0, 2);
@@ -270,6 +297,10 @@ static SENSOR_DEVICE_ATTR_2(in6_input, 0444, adc128_show_in, NULL, 6, 0);
 static SENSOR_DEVICE_ATTR_2(in6_min, 0644, adc128_show_in, adc128_set_in, 6, 1);
 static SENSOR_DEVICE_ATTR_2(in6_max, 0644, adc128_show_in, adc128_set_in, 6, 2);
 
+static SENSOR_DEVICE_ATTR_2(in7_input, 0444, adc128_show_in, NULL, 7, 0);
+static SENSOR_DEVICE_ATTR_2(in7_min, 0644, adc128_show_in, adc128_set_in, 7, 1);
+static SENSOR_DEVICE_ATTR_2(in7_max, 0644, adc128_show_in, adc128_set_in, 7, 2);
+
 static SENSOR_DEVICE_ATTR(temp1_input, 0444, adc128_show_temp, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp1_max, 0644,
 			  adc128_show_temp, adc128_set_temp, 1);
@@ -283,44 +314,54 @@ static SENSOR_DEVICE_ATTR(in3_alarm, 0444, adc128_show_alarm, NULL, 3);
 static SENSOR_DEVICE_ATTR(in4_alarm, 0444, adc128_show_alarm, NULL, 4);
 static SENSOR_DEVICE_ATTR(in5_alarm, 0444, adc128_show_alarm, NULL, 5);
 static SENSOR_DEVICE_ATTR(in6_alarm, 0444, adc128_show_alarm, NULL, 6);
+static SENSOR_DEVICE_ATTR(in7_alarm, 0444, adc128_show_alarm, NULL, 7);
 static SENSOR_DEVICE_ATTR(temp1_max_alarm, 0444, adc128_show_alarm, NULL, 7);
 
 static struct attribute *adc128_attrs[] = {
-	&sensor_dev_attr_in0_min.dev_attr.attr,
-	&sensor_dev_attr_in1_min.dev_attr.attr,
-	&sensor_dev_attr_in2_min.dev_attr.attr,
-	&sensor_dev_attr_in3_min.dev_attr.attr,
-	&sensor_dev_attr_in4_min.dev_attr.attr,
-	&sensor_dev_attr_in5_min.dev_attr.attr,
-	&sensor_dev_attr_in6_min.dev_attr.attr,
-	&sensor_dev_attr_in0_max.dev_attr.attr,
-	&sensor_dev_attr_in1_max.dev_attr.attr,
-	&sensor_dev_attr_in2_max.dev_attr.attr,
-	&sensor_dev_attr_in3_max.dev_attr.attr,
-	&sensor_dev_attr_in4_max.dev_attr.attr,
-	&sensor_dev_attr_in5_max.dev_attr.attr,
-	&sensor_dev_attr_in6_max.dev_attr.attr,
+	&sensor_dev_attr_in0_alarm.dev_attr.attr,
 	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in0_max.dev_attr.attr,
+	&sensor_dev_attr_in0_min.dev_attr.attr,
+	&sensor_dev_attr_in1_alarm.dev_attr.attr,
 	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in1_max.dev_attr.attr,
+	&sensor_dev_attr_in1_min.dev_attr.attr,
+	&sensor_dev_attr_in2_alarm.dev_attr.attr,
 	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in2_max.dev_attr.attr,
+	&sensor_dev_attr_in2_min.dev_attr.attr,
+	&sensor_dev_attr_in3_alarm.dev_attr.attr,
 	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in3_max.dev_attr.attr,
+	&sensor_dev_attr_in3_min.dev_attr.attr,
+	&sensor_dev_attr_in4_alarm.dev_attr.attr,
 	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in4_max.dev_attr.attr,
+	&sensor_dev_attr_in4_min.dev_attr.attr,
+	&sensor_dev_attr_in5_alarm.dev_attr.attr,
 	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in5_max.dev_attr.attr,
+	&sensor_dev_attr_in5_min.dev_attr.attr,
+	&sensor_dev_attr_in6_alarm.dev_attr.attr,
 	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in6_max.dev_attr.attr,
+	&sensor_dev_attr_in6_min.dev_attr.attr,
+	&sensor_dev_attr_in7_alarm.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+	&sensor_dev_attr_in7_max.dev_attr.attr,
+	&sensor_dev_attr_in7_min.dev_attr.attr,
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp1_max.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_in0_alarm.dev_attr.attr,
-	&sensor_dev_attr_in1_alarm.dev_attr.attr,
-	&sensor_dev_attr_in2_alarm.dev_attr.attr,
-	&sensor_dev_attr_in3_alarm.dev_attr.attr,
-	&sensor_dev_attr_in4_alarm.dev_attr.attr,
-	&sensor_dev_attr_in5_alarm.dev_attr.attr,
-	&sensor_dev_attr_in6_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(adc128);
+
+static struct attribute_group adc128_group = {
+	.attrs = adc128_attrs,
+	.is_visible = adc128_is_visible,
+};
+__ATTRIBUTE_GROUPS(adc128);
 
 static int adc128_detect(struct i2c_client *client, struct i2c_board_info *info)
 {
@@ -425,9 +466,8 @@ static int adc128_probe(struct i2c_client *client,
 
 	/* Operation mode is optional and defaults to mode 0 */
 	if (of_property_read_u8(dev->of_node, "mode", &data->mode) == 0) {
-		/* Currently only mode 0 supported */
-		if (data->mode != 0) {
-			dev_err(dev, "unsupported operation mode %d",
+		if (data->mode > 3) {
+			dev_err(dev, "invalid operation mode %d",
 				data->mode);
 			err = -EINVAL;
 			goto error;
-- 
2.11.0

^ permalink raw reply related

* [PATCH] adc: add adc driver for Hisilicon BVT SOCs
From: Allen Liu @ 2016-12-24  1:54 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w,
	ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w,
	krzk-DgEjT+Ai2ygdnm+yROfE0A, vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w,
	ksenija.stanojevic-Re5JQEeQqe8AvxtiuMwx3w,
	zhiyong.tao-NuS5LvNUpcJWk0Htik3J/w,
	daniel.baluta-ral2JQCrhuEAvxtiuMwx3w,
	leonard.crestez-ral2JQCrhuEAvxtiuMwx3w,
	ray.jui-dY08KVG/lbpWk0Htik3J/w,
	raveendra.padasalagi-dY08KVG/lbpWk0Htik3J/w,
	mranostay-Re5JQEeQqe8AvxtiuMwx3w,
	amsfield22-Re5JQEeQqe8AvxtiuMwx3w,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
	kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q,
	liurenzhong-C8/M+/jPZTeaMJb+Lgu22Q

Add ADC driver for the ADC controller found on HiSilicon BVT SOCs, like Hi3516CV300, etc.
The ADC controller is primarily in charge of detecting voltage.

Reviewed-by: Jiancheng Xue <xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Allen Liu <liurenzhong-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/hibvt-lsadc.txt    |  26 ++
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/hibvt_lsadc.c                      | 344 +++++++++++++++++++++
 4 files changed, 381 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt
 create mode 100644 drivers/iio/adc/hibvt_lsadc.c

diff --git a/Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt b/Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt
new file mode 100644
index 0000000..63de46e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt
@@ -0,0 +1,26 @@
+Hisilicon BVT Low Speed (LS) A/D Converter bindings
+
+Required properties:
+- compatible: should be "hisilicon,<name>-lsadc"
+   - "hisilicon,hibvt-lsadc": for hi3516cv300
+
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number to the cpu. The interrupt specifier format
+              depends on the interrupt controller.
+- #io-channel-cells: Should be 1, see ../iio-bindings.txt
+
+Optional properties:
+- resets: Must contain an entry for each entry in reset-names if need support
+	  this option. See ../reset/reset.txt for details.
+- reset-names: Must include the name "saradc-apb".
+
+Example:
+	lsadc: hibvt-lsadc@120e0000 {
+			compatible = "hisilicon,hibvt-lsadc";
+			reg = <0x120e0000 0x1000>;
+			interrupts = <19>;
+			resets = <&crg 0x7c 3>;
+			reset-names = "lsadc-crg";
+			status = "disabled";
+	};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..0443f51 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -225,6 +225,16 @@ config HI8435
 	  This driver can also be built as a module. If so, the module will be
 	  called hi8435.
 
+config HIBVT_LSADC
+	tristate "HIBVT LSADC driver"
+	depends on ARCH_HISI || COMPILE_TEST
+	help
+	  Say yes here to build support for the LSADC found in SoCs from
+	  hisilicon BVT chip.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hibvt_lsadc.
+
 config INA2XX_ADC
 	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
 	depends on I2C && !SENSORS_INA2XX
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04..6554d92 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
 obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_HI8435) += hi8435.o
+obj-$(CONFIG_HIBVT_LSADC) += hibvt_lsadc.o
 obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
 obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
diff --git a/drivers/iio/adc/hibvt_lsadc.c b/drivers/iio/adc/hibvt_lsadc.c
new file mode 100644
index 0000000..a20afe8
--- /dev/null
+++ b/drivers/iio/adc/hibvt_lsadc.c
@@ -0,0 +1,344 @@
+/*
+ * Hisilicon BVT Low Speed (LS) A/D Converter
+ * Copyright (C) 2016 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/reset.h>
+#include <linux/regulator/consumer.h>
+#include <linux/iio/iio.h>
+
+/* hisilicon bvt adc registers definitions */
+#define LSADC_CONFIG		0x00
+#define CONFIG_DEGLITCH		BIT(17)
+#define CONFIG_RESET		BIT(15)
+#define CONFIG_POWERDOWN	BIT(14)
+#define CONFIG_MODE			BIT(13)
+#define CONFIG_CHC_VALID	BIT(10)
+#define CONFIG_CHB_VALID	BIT(9)
+#define CONFIG_CHA_VALID	BIT(8)
+
+#define LSADC_TIMESCAN		0x08
+#define LSADC_INTEN			0x10
+#define LSADC_INTSTATUS		0x14
+#define LSADC_INTCLR		0x18
+#define LSADC_START			0x1C
+#define LSADC_STOP			0x20
+#define LSADC_ACTBIT		0x24
+#define LSADC_CHNDATA		0x2C
+
+#define ADC_CON_EN			(1u << 0)
+#define ADC_CON_DEN			(0u << 0)
+
+#define ADC_NUM_BITS		10
+
+/* fix clk:3000000, default tscan set 10ms */
+#define DEF_ADC_TSCAN_MS	(10*3000)
+
+#define LSADC_CHN_MASK		0x7
+
+#define LSADC_TIMEOUT		msecs_to_jiffies(100)
+
+/* default voltage scale for every channel <mv> */
+static int g_voltage[] = {
+	3300, 3300, 3300
+};
+
+struct hibvt_lsadc {
+	void __iomem		*regs;
+	struct completion	completion;
+	struct reset_control	*reset;
+	const struct hibvt_lsadc_data	*data;
+	unsigned int		cur_chn;
+	unsigned int		value;
+};
+
+struct hibvt_lsadc_data {
+	int				num_bits;
+	const struct iio_chan_spec	*channels;
+	int				num_channels;
+
+	void (*clear_irq)(struct hibvt_lsadc *info, int mask);
+	void (*start_conv)(struct hibvt_lsadc *info);
+	void (*stop_conv)(struct hibvt_lsadc *info);
+};
+
+static int hibvt_lsadc_read_raw(struct iio_dev *indio_dev,
+				    struct iio_chan_spec const *chan,
+				    int *val, int *val2, long mask)
+{
+	struct hibvt_lsadc *info = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+
+		reinit_completion(&info->completion);
+
+		/* Select the channel to be used */
+		info->cur_chn = chan->channel;
+
+		if (info->data->start_conv)
+			info->data->start_conv(info);
+
+		if (!wait_for_completion_timeout(&info->completion,
+							LSADC_TIMEOUT)) {
+			if (info->data->stop_conv)
+				info->data->stop_conv(info);
+			mutex_unlock(&indio_dev->mlock);
+			return -ETIMEDOUT;
+		}
+
+		*val = info->value;
+		mutex_unlock(&indio_dev->mlock);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = g_voltage[chan->channel];
+		*val2 = info->data->num_bits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static irqreturn_t hibvt_lsadc_isr(int irq, void *dev_id)
+{
+	struct hibvt_lsadc *info = (struct hibvt_lsadc *)dev_id;
+	int mask;
+
+	mask = readl(info->regs + LSADC_INTSTATUS);
+	mask &= LSADC_CHN_MASK;
+
+	/* Clear irq */
+	if (info->data->clear_irq)
+		info->data->clear_irq(info, mask);
+
+	/* Read value */
+	info->value = readl(info->regs + LSADC_CHNDATA + (info->cur_chn << 2));
+	info->value &= GENMASK(info->data->num_bits - 1, 0);
+
+	/* stop adc */
+	if (info->data->stop_conv)
+		info->data->stop_conv(info);
+
+	complete(&info->completion);
+
+	return IRQ_HANDLED;
+}
+
+static const struct iio_info hibvt_lsadc_iio_info = {
+	.read_raw = hibvt_lsadc_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define ADC_CHANNEL(_index, _id) {      \
+	.type = IIO_VOLTAGE,                \
+	.indexed = 1,						\
+	.channel = _index,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
+			BIT(IIO_CHAN_INFO_SCALE),   \
+	.datasheet_name = _id,              \
+}
+
+static const struct iio_chan_spec hibvt_lsadc_iio_channels[] = {
+	ADC_CHANNEL(0, "adc0"),
+	ADC_CHANNEL(1, "adc1"),
+	ADC_CHANNEL(2, "adc2"),
+};
+
+static void hibvt_lsadc_clear_irq(struct hibvt_lsadc *info, int mask)
+{
+	writel(mask, info->regs + LSADC_INTCLR);
+}
+
+static void hibvt_lsadc_start_conv(struct hibvt_lsadc *info)
+{
+	unsigned int con;
+
+	/* set number bit */
+	con = GENMASK(info->data->num_bits - 1, 0);
+	writel(con, (info->regs + LSADC_ACTBIT));
+
+    /* config */
+	con = readl(info->regs + LSADC_CONFIG);
+	con &= ~CONFIG_RESET;
+	con |= (CONFIG_POWERDOWN | CONFIG_DEGLITCH | CONFIG_MODE);
+	con &= ~(CONFIG_CHA_VALID | CONFIG_CHB_VALID | CONFIG_CHC_VALID);
+	con |= (CONFIG_CHA_VALID << info->cur_chn);
+	writel(con, (info->regs + LSADC_CONFIG));
+
+	/* set timescan */
+	writel(DEF_ADC_TSCAN_MS, (info->regs + LSADC_TIMESCAN));
+
+	/* clear interrupt */
+	writel(LSADC_CHN_MASK, info->regs + LSADC_INTCLR);
+
+	/* enable interrupt */
+	writel(ADC_CON_EN, (info->regs + LSADC_INTEN));
+
+	/* start scan */
+	writel(ADC_CON_EN, (info->regs + LSADC_START));
+}
+
+static void hibvt_lsadc_stop_conv(struct hibvt_lsadc *info)
+{
+	/* reset the timescan */
+	writel(ADC_CON_DEN, (info->regs + LSADC_TIMESCAN));
+
+	/* disable interrupt */
+	writel(ADC_CON_DEN, (info->regs + LSADC_INTEN));
+
+	/* stop scan */
+	writel(ADC_CON_EN, (info->regs + LSADC_STOP));
+}
+
+static const struct hibvt_lsadc_data lsadc_data = {
+	.num_bits = ADC_NUM_BITS,
+	.channels = hibvt_lsadc_iio_channels,
+	.num_channels = ARRAY_SIZE(hibvt_lsadc_iio_channels),
+
+	.clear_irq	= hibvt_lsadc_clear_irq,
+	.start_conv	= hibvt_lsadc_start_conv,
+	.stop_conv = hibvt_lsadc_stop_conv,
+};
+
+static const struct of_device_id hibvt_lsadc_match[] = {
+	{
+		.compatible = "hisilicon,hibvt-lsadc",
+		.data = &lsadc_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, hibvt_lsadc_match);
+
+/**
+ * Reset LSADC Controller.
+ */
+static void hibvt_lsadc_reset_controller(struct reset_control *reset)
+{
+	reset_control_assert(reset);
+	usleep_range(10, 20);
+	reset_control_deassert(reset);
+}
+
+static int hibvt_lsadc_probe(struct platform_device *pdev)
+{
+	struct hibvt_lsadc *info = NULL;
+	struct device_node *np = pdev->dev.of_node;
+	struct iio_dev *indio_dev = NULL;
+	struct resource	*mem;
+	const struct of_device_id *match;
+	int ret;
+	int irq;
+
+	if (!np)
+		return -ENODEV;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
+	if (!indio_dev) {
+		dev_err(&pdev->dev, "failed allocating iio device\n");
+		return -ENOMEM;
+	}
+	info = iio_priv(indio_dev);
+
+	match = of_match_device(hibvt_lsadc_match, &pdev->dev);
+	info->data = match->data;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	info->regs = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(info->regs))
+		return PTR_ERR(info->regs);
+
+	/*
+	 * The reset should be an optional property, as it should work
+	 * with old devicetrees as well
+	 */
+	info->reset = devm_reset_control_get(&pdev->dev, "lsadc-crg");
+	if (IS_ERR(info->reset)) {
+		ret = PTR_ERR(info->reset);
+		if (ret != -ENOENT)
+			return ret;
+
+		dev_dbg(&pdev->dev, "no reset control found\n");
+		info->reset = NULL;
+	}
+
+	init_completion(&info->completion);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq resource?\n");
+		return irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, hibvt_lsadc_isr,
+			       0, dev_name(&pdev->dev), info);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed requesting irq %d\n", irq);
+		return ret;
+	}
+
+	if (info->reset)
+		hibvt_lsadc_reset_controller(info->reset);
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	indio_dev->name = dev_name(&pdev->dev);
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->dev.of_node = pdev->dev.of_node;
+	indio_dev->info = &hibvt_lsadc_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	indio_dev->channels = info->data->channels;
+	indio_dev->num_channels = info->data->num_channels;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed register iio device\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int hibvt_lsadc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static struct platform_driver hibvt_lsadc_driver = {
+	.probe		= hibvt_lsadc_probe,
+	.remove		= hibvt_lsadc_remove,
+	.driver		= {
+		.name	= "hibvt-lsadc",
+		.of_match_table = hibvt_lsadc_match,
+	},
+};
+
+module_platform_driver(hibvt_lsadc_driver);
+
+MODULE_AUTHOR("Allen Liu <liurenzhong-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>");
+MODULE_DESCRIPTION("hisilicon BVT LSADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

^ permalink raw reply related

* Re: [PATCH v2 4/4] clk: rockchip: add new pll-type for rk3328
From: Heiko Stuebner @ 2016-12-24  2:18 UTC (permalink / raw)
  To: Elaine Zhang
  Cc: mturquette, sboyd, xf, robh+dt, mark.rutland, linux-clk,
	linux-arm-kernel, devicetree, huangtao, xxx, cl, linux-rockchip,
	linux-kernel
In-Reply-To: <1482112573-11613-5-git-send-email-zhangqing@rock-chips.com>

Hi Elaine,

Am Montag, 19. Dezember 2016, 09:56:13 CET schrieb Elaine Zhang:
> The rk3328's pll and clock are similar with rk3036's,
> it different with pll_mode_mask,there are different
> control registers bit,
> so these should be independent and separate from
> the series of rk3328s.

not sure I understand this description. In the patch (and TRM excerpt) I see
that the number of parents is down to only xin24m but the general handling is
similar to the rk3036 and thus reuses its operations.

The description makes it sound like there are operational differences (the
"different control register bit" part), so could you clarify this a bit
please?

Also, please move the pll addition before the addition of the clock-controller
in the series and move the pll-type addition also here from the controller
patch.

Some more below:

> 
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---
>  drivers/clk/rockchip/clk-pll.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
> index 6ed605776abd..9650c75f61d1 100644
> --- a/drivers/clk/rockchip/clk-pll.c
> +++ b/drivers/clk/rockchip/clk-pll.c
> @@ -29,6 +29,7 @@
>  #define PLL_MODE_SLOW		0x0
>  #define PLL_MODE_NORM		0x1
>  #define PLL_MODE_DEEP		0x2
> +#define PLL_RK3328_MODE_MASK	0x1
> 
>  struct rockchip_clk_pll {
>  	struct clk_hw		hw;
> @@ -865,13 +866,17 @@ struct clk *rockchip_clk_register_pll(struct
> rockchip_clk_provider *ctx, pll_mux = &pll->pll_mux;
>  	pll_mux->reg = ctx->reg_base + mode_offset;
>  	pll_mux->shift = mode_shift;
> -	pll_mux->mask = PLL_MODE_MASK;
> +	if (pll_type == pll_rk3328)
> +		pll_mux->mask = PLL_RK3328_MODE_MASK;
> +	else
> +		pll_mux->mask = PLL_MODE_MASK;

you're missing the other parts handling parents, like num_parents check
and the init.num_parents parameter.

The pll really has only one parent, xin24m, so we should handle this
correctly in the code, instead of having 2 times xin24m in the parent
array coming from the clock controller.

>  	pll_mux->flags = 0;
>  	pll_mux->lock = &ctx->lock;
>  	pll_mux->hw.init = &init;
> 
>  	if (pll_type == pll_rk3036 ||
>  	    pll_type == pll_rk3066 ||
> +	    pll_type == pll_rk3328 ||
>  	    pll_type == pll_rk3399)
>  		pll_mux->flags |= CLK_MUX_HIWORD_MASK;
> 
> @@ -929,6 +934,12 @@ struct clk *rockchip_clk_register_pll(struct
> rockchip_clk_provider *ctx, else
>  			init.ops = &rockchip_rk3066_pll_clk_ops;
>  		break;
> +	case pll_rk3328:
> +		if (!pll->rate_table || IS_ERR(ctx->grf))
> +			init.ops = &rockchip_rk3036_pll_clk_norate_ops;
> +		else
> +			init.ops = &rockchip_rk3036_pll_clk_ops;
> +		break;

please don't duplicate the rk3036-ops assignment, when adding the
pll_rk3328 option to the rk3036 part suffices.

I'd think the pll-patch should look something like the
following (untested, so please test):
--------------- 8< -----------------
Subject: [PATCH] clk: rockchip: add new pll-type for rk3328

The rk3328's pll and clock are similar with rk3036's,
it different with pll_mode_mask,there are different
control registers bit,
so these should be independent and separate from
the series of rk3328s.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-pll.c | 18 ++++++++++++++----
 drivers/clk/rockchip/clk.h     |  1 +
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index 6ed6057..99ce483 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -29,6 +29,7 @@
 #define PLL_MODE_SLOW		0x0
 #define PLL_MODE_NORM		0x1
 #define PLL_MODE_DEEP		0x2
+#define PLL_RK3328_MODE_MASK	0x1
 
 struct rockchip_clk_pll {
 	struct clk_hw		hw;
@@ -848,8 +849,9 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
 	struct clk *pll_clk, *mux_clk;
 	char pll_name[20];
 
-	if (num_parents != 2) {
-		pr_err("%s: needs two parent clocks\n", __func__);
+	if ((pll_type != pll_rk3328 && num_parents != 2) ||
+	    (pll_type == pll_rk3328 && num_parents != 1)) {
+		pr_err("%s: missing parent clocks\n", __func__);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -865,13 +867,17 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
 	pll_mux = &pll->pll_mux;
 	pll_mux->reg = ctx->reg_base + mode_offset;
 	pll_mux->shift = mode_shift;
-	pll_mux->mask = PLL_MODE_MASK;
+	if (pll_type == pll_rk3328)
+		pll_mux->mask = PLL_RK3328_MODE_MASK;
+	else
+		pll_mux->mask = PLL_MODE_MASK;
 	pll_mux->flags = 0;
 	pll_mux->lock = &ctx->lock;
 	pll_mux->hw.init = &init;
 
 	if (pll_type == pll_rk3036 ||
 	    pll_type == pll_rk3066 ||
+	    pll_type == pll_rk3328 ||
 	    pll_type == pll_rk3399)
 		pll_mux->flags |= CLK_MUX_HIWORD_MASK;
 
@@ -884,7 +890,10 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
 	init.flags = CLK_SET_RATE_PARENT;
 	init.ops = pll->pll_mux_ops;
 	init.parent_names = pll_parents;
-	init.num_parents = ARRAY_SIZE(pll_parents);
+	if (pll_type == pll_rk3328)
+		init.num_parents = 2;
+	else
+		init.num_parents = ARRAY_SIZE(pll_parents);
 
 	mux_clk = clk_register(NULL, &pll_mux->hw);
 	if (IS_ERR(mux_clk))
@@ -918,6 +927,7 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
 
 	switch (pll_type) {
 	case pll_rk3036:
+	case pll_rk3328:
 		if (!pll->rate_table || IS_ERR(ctx->grf))
 			init.ops = &rockchip_rk3036_pll_clk_norate_ops;
 		else
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index d67eecc..06acb7e 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -130,6 +130,7 @@ struct clk;
 enum rockchip_pll_type {
 	pll_rk3036,
 	pll_rk3066,
+	pll_rk3328,
 	pll_rk3399,
 };
 
-- 
2.10.2

--------------- 8< -----------------


^ permalink raw reply related

* Re: [PATCH] adc: add adc driver for Hisilicon BVT SOCs
From: Jiancheng Xue @ 2016-12-24  2:31 UTC (permalink / raw)
  To: Allen Liu, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q,
	hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q,
	akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w,
	ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w,
	krzk-DgEjT+Ai2ygdnm+yROfE0A, vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w,
	ksenija.stanojevic-Re5JQEeQqe8AvxtiuMwx3w,
	zhiyong.tao-NuS5LvNUpcJWk0Htik3J/w,
	daniel.baluta-ral2JQCrhuEAvxtiuMwx3w,
	leonard.crestez-ral2JQCrhuEAvxtiuMwx3w,
	ray.jui-dY08KVG/lbpWk0Htik3J/w,
	raveendra.padasalagi-dY08KVG/lbpWk0Htik3J/w,
	mranostay-Re5JQEeQqe8AvxtiuMwx3w,
	amsfield22-Re5JQEeQqe8AvxtiuMwx3w,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <1482544497-136656-1-git-send-email-liurenzhong-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>



On 2016/12/24 9:54, Allen Liu wrote:
> Add ADC driver for the ADC controller found on HiSilicon BVT SOCs, like Hi3516CV300, etc.
> The ADC controller is primarily in charge of detecting voltage.
> 
> Reviewed-by: Jiancheng Xue <xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Hi

Sorry. I haven't reviewed this patch. Please remove this line. Thank you!

Regards,
Jiancheng

> Signed-off-by: Allen Liu <liurenzhong-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/adc/hibvt-lsadc.txt    |  26 ++
>  drivers/iio/adc/Kconfig                            |  10 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/hibvt_lsadc.c                      | 344 +++++++++++++++++++++
>  4 files changed, 381 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt
>  create mode 100644 drivers/iio/adc/hibvt_lsadc.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt b/Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt
> new file mode 100644
> index 0000000..63de46e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/hibvt-lsadc.txt
> @@ -0,0 +1,26 @@
> +Hisilicon BVT Low Speed (LS) A/D Converter bindings
> +
> +Required properties:
> +- compatible: should be "hisilicon,<name>-lsadc"
> +   - "hisilicon,hibvt-lsadc": for hi3516cv300
> +
> +- reg: physical base address of the controller and length of memory mapped
> +       region.
> +- interrupts: The interrupt number to the cpu. The interrupt specifier format
> +              depends on the interrupt controller.
> +- #io-channel-cells: Should be 1, see ../iio-bindings.txt
> +
> +Optional properties:
> +- resets: Must contain an entry for each entry in reset-names if need support
> +	  this option. See ../reset/reset.txt for details.
> +- reset-names: Must include the name "saradc-apb".
> +
> +Example:
> +	lsadc: hibvt-lsadc@120e0000 {
> +			compatible = "hisilicon,hibvt-lsadc";
> +			reg = <0x120e0000 0x1000>;
> +			interrupts = <19>;
> +			resets = <&crg 0x7c 3>;
> +			reset-names = "lsadc-crg";
> +			status = "disabled";
> +	};
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..0443f51 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -225,6 +225,16 @@ config HI8435
>  	  This driver can also be built as a module. If so, the module will be
>  	  called hi8435.
>  
> +config HIBVT_LSADC
> +	tristate "HIBVT LSADC driver"
> +	depends on ARCH_HISI || COMPILE_TEST
> +	help
> +	  Say yes here to build support for the LSADC found in SoCs from
> +	  hisilicon BVT chip.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called hibvt_lsadc.
> +
>  config INA2XX_ADC
>  	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
>  	depends on I2C && !SENSORS_INA2XX
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7a40c04..6554d92 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
>  obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
>  obj-$(CONFIG_HI8435) += hi8435.o
> +obj-$(CONFIG_HIBVT_LSADC) += hibvt_lsadc.o
>  obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
>  obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> diff --git a/drivers/iio/adc/hibvt_lsadc.c b/drivers/iio/adc/hibvt_lsadc.c
> new file mode 100644
> index 0000000..a20afe8
> --- /dev/null
> +++ b/drivers/iio/adc/hibvt_lsadc.c
> @@ -0,0 +1,344 @@
> +/*
> + * Hisilicon BVT Low Speed (LS) A/D Converter
> + * Copyright (C) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/reset.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/iio/iio.h>
> +
> +/* hisilicon bvt adc registers definitions */
> +#define LSADC_CONFIG		0x00
> +#define CONFIG_DEGLITCH		BIT(17)
> +#define CONFIG_RESET		BIT(15)
> +#define CONFIG_POWERDOWN	BIT(14)
> +#define CONFIG_MODE			BIT(13)
> +#define CONFIG_CHC_VALID	BIT(10)
> +#define CONFIG_CHB_VALID	BIT(9)
> +#define CONFIG_CHA_VALID	BIT(8)
> +
> +#define LSADC_TIMESCAN		0x08
> +#define LSADC_INTEN			0x10
> +#define LSADC_INTSTATUS		0x14
> +#define LSADC_INTCLR		0x18
> +#define LSADC_START			0x1C
> +#define LSADC_STOP			0x20
> +#define LSADC_ACTBIT		0x24
> +#define LSADC_CHNDATA		0x2C
> +
> +#define ADC_CON_EN			(1u << 0)
> +#define ADC_CON_DEN			(0u << 0)
> +
> +#define ADC_NUM_BITS		10
> +
> +/* fix clk:3000000, default tscan set 10ms */
> +#define DEF_ADC_TSCAN_MS	(10*3000)
> +
> +#define LSADC_CHN_MASK		0x7
> +
> +#define LSADC_TIMEOUT		msecs_to_jiffies(100)
> +
> +/* default voltage scale for every channel <mv> */
> +static int g_voltage[] = {
> +	3300, 3300, 3300
> +};
> +
> +struct hibvt_lsadc {
> +	void __iomem		*regs;
> +	struct completion	completion;
> +	struct reset_control	*reset;
> +	const struct hibvt_lsadc_data	*data;
> +	unsigned int		cur_chn;
> +	unsigned int		value;
> +};
> +
> +struct hibvt_lsadc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +
> +	void (*clear_irq)(struct hibvt_lsadc *info, int mask);
> +	void (*start_conv)(struct hibvt_lsadc *info);
> +	void (*stop_conv)(struct hibvt_lsadc *info);
> +};
> +
> +static int hibvt_lsadc_read_raw(struct iio_dev *indio_dev,
> +				    struct iio_chan_spec const *chan,
> +				    int *val, int *val2, long mask)
> +{
> +	struct hibvt_lsadc *info = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&indio_dev->mlock);
> +
> +		reinit_completion(&info->completion);
> +
> +		/* Select the channel to be used */
> +		info->cur_chn = chan->channel;
> +
> +		if (info->data->start_conv)
> +			info->data->start_conv(info);
> +
> +		if (!wait_for_completion_timeout(&info->completion,
> +							LSADC_TIMEOUT)) {
> +			if (info->data->stop_conv)
> +				info->data->stop_conv(info);
> +			mutex_unlock(&indio_dev->mlock);
> +			return -ETIMEDOUT;
> +		}
> +
> +		*val = info->value;
> +		mutex_unlock(&indio_dev->mlock);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = g_voltage[chan->channel];
> +		*val2 = info->data->num_bits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static irqreturn_t hibvt_lsadc_isr(int irq, void *dev_id)
> +{
> +	struct hibvt_lsadc *info = (struct hibvt_lsadc *)dev_id;
> +	int mask;
> +
> +	mask = readl(info->regs + LSADC_INTSTATUS);
> +	mask &= LSADC_CHN_MASK;
> +
> +	/* Clear irq */
> +	if (info->data->clear_irq)
> +		info->data->clear_irq(info, mask);
> +
> +	/* Read value */
> +	info->value = readl(info->regs + LSADC_CHNDATA + (info->cur_chn << 2));
> +	info->value &= GENMASK(info->data->num_bits - 1, 0);
> +
> +	/* stop adc */
> +	if (info->data->stop_conv)
> +		info->data->stop_conv(info);
> +
> +	complete(&info->completion);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info hibvt_lsadc_iio_info = {
> +	.read_raw = hibvt_lsadc_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +#define ADC_CHANNEL(_index, _id) {      \
> +	.type = IIO_VOLTAGE,                \
> +	.indexed = 1,						\
> +	.channel = _index,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
> +			BIT(IIO_CHAN_INFO_SCALE),   \
> +	.datasheet_name = _id,              \
> +}
> +
> +static const struct iio_chan_spec hibvt_lsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +	ADC_CHANNEL(2, "adc2"),
> +};
> +
> +static void hibvt_lsadc_clear_irq(struct hibvt_lsadc *info, int mask)
> +{
> +	writel(mask, info->regs + LSADC_INTCLR);
> +}
> +
> +static void hibvt_lsadc_start_conv(struct hibvt_lsadc *info)
> +{
> +	unsigned int con;
> +
> +	/* set number bit */
> +	con = GENMASK(info->data->num_bits - 1, 0);
> +	writel(con, (info->regs + LSADC_ACTBIT));
> +
> +    /* config */
> +	con = readl(info->regs + LSADC_CONFIG);
> +	con &= ~CONFIG_RESET;
> +	con |= (CONFIG_POWERDOWN | CONFIG_DEGLITCH | CONFIG_MODE);
> +	con &= ~(CONFIG_CHA_VALID | CONFIG_CHB_VALID | CONFIG_CHC_VALID);
> +	con |= (CONFIG_CHA_VALID << info->cur_chn);
> +	writel(con, (info->regs + LSADC_CONFIG));
> +
> +	/* set timescan */
> +	writel(DEF_ADC_TSCAN_MS, (info->regs + LSADC_TIMESCAN));
> +
> +	/* clear interrupt */
> +	writel(LSADC_CHN_MASK, info->regs + LSADC_INTCLR);
> +
> +	/* enable interrupt */
> +	writel(ADC_CON_EN, (info->regs + LSADC_INTEN));
> +
> +	/* start scan */
> +	writel(ADC_CON_EN, (info->regs + LSADC_START));
> +}
> +
> +static void hibvt_lsadc_stop_conv(struct hibvt_lsadc *info)
> +{
> +	/* reset the timescan */
> +	writel(ADC_CON_DEN, (info->regs + LSADC_TIMESCAN));
> +
> +	/* disable interrupt */
> +	writel(ADC_CON_DEN, (info->regs + LSADC_INTEN));
> +
> +	/* stop scan */
> +	writel(ADC_CON_EN, (info->regs + LSADC_STOP));
> +}
> +
> +static const struct hibvt_lsadc_data lsadc_data = {
> +	.num_bits = ADC_NUM_BITS,
> +	.channels = hibvt_lsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(hibvt_lsadc_iio_channels),
> +
> +	.clear_irq	= hibvt_lsadc_clear_irq,
> +	.start_conv	= hibvt_lsadc_start_conv,
> +	.stop_conv = hibvt_lsadc_stop_conv,
> +};
> +
> +static const struct of_device_id hibvt_lsadc_match[] = {
> +	{
> +		.compatible = "hisilicon,hibvt-lsadc",
> +		.data = &lsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, hibvt_lsadc_match);
> +
> +/**
> + * Reset LSADC Controller.
> + */
> +static void hibvt_lsadc_reset_controller(struct reset_control *reset)
> +{
> +	reset_control_assert(reset);
> +	usleep_range(10, 20);
> +	reset_control_deassert(reset);
> +}
> +
> +static int hibvt_lsadc_probe(struct platform_device *pdev)
> +{
> +	struct hibvt_lsadc *info = NULL;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct iio_dev *indio_dev = NULL;
> +	struct resource	*mem;
> +	const struct of_device_id *match;
> +	int ret;
> +	int irq;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> +	if (!indio_dev) {
> +		dev_err(&pdev->dev, "failed allocating iio device\n");
> +		return -ENOMEM;
> +	}
> +	info = iio_priv(indio_dev);
> +
> +	match = of_match_device(hibvt_lsadc_match, &pdev->dev);
> +	info->data = match->data;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> +	if (IS_ERR(info->regs))
> +		return PTR_ERR(info->regs);
> +
> +	/*
> +	 * The reset should be an optional property, as it should work
> +	 * with old devicetrees as well
> +	 */
> +	info->reset = devm_reset_control_get(&pdev->dev, "lsadc-crg");
> +	if (IS_ERR(info->reset)) {
> +		ret = PTR_ERR(info->reset);
> +		if (ret != -ENOENT)
> +			return ret;
> +
> +		dev_dbg(&pdev->dev, "no reset control found\n");
> +		info->reset = NULL;
> +	}
> +
> +	init_completion(&info->completion);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "no irq resource?\n");
> +		return irq;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, irq, hibvt_lsadc_isr,
> +			       0, dev_name(&pdev->dev), info);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed requesting irq %d\n", irq);
> +		return ret;
> +	}
> +
> +	if (info->reset)
> +		hibvt_lsadc_reset_controller(info->reset);
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	indio_dev->name = dev_name(&pdev->dev);
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->dev.of_node = pdev->dev.of_node;
> +	indio_dev->info = &hibvt_lsadc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed register iio device\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hibvt_lsadc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver hibvt_lsadc_driver = {
> +	.probe		= hibvt_lsadc_probe,
> +	.remove		= hibvt_lsadc_remove,
> +	.driver		= {
> +		.name	= "hibvt-lsadc",
> +		.of_match_table = hibvt_lsadc_match,
> +	},
> +};
> +
> +module_platform_driver(hibvt_lsadc_driver);
> +
> +MODULE_AUTHOR("Allen Liu <liurenzhong-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>");
> +MODULE_DESCRIPTION("hisilicon BVT LSADC driver");
> +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply

* [PATCH] ARM: dts: imx: remove obsoleted property fsl,spi-num-chipselects
From: Vladimir Zapolskiy @ 2016-12-24  5:43 UTC (permalink / raw)
  To: Shawn Guo, Fabio Estevam
  Cc: Sascha Hauer, Alexander Shiyan, Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Since commit b36581df7e78 ("spi: imx: Using existing properties for
chipselects") the device tree property 'fsl,spi-num-chipselects' is
unused and it is already marked as obsolete in device tree binding
documentation. Remove the property from the existing DTS files to
avoid its reoccurence on copying.

Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/imx1-ads.dts                         | 1 -
 arch/arm/boot/dts/imx27-apf27dev.dts                   | 2 --
 arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts | 1 -
 arch/arm/boot/dts/imx27-pdk.dts                        | 1 -
 arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi      | 1 -
 arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts         | 1 -
 arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi        | 1 -
 arch/arm/boot/dts/imx50-evk.dts                        | 1 -
 arch/arm/boot/dts/imx51-apf51dev.dts                   | 2 --
 arch/arm/boot/dts/imx51-babbage.dts                    | 1 -
 arch/arm/boot/dts/imx51-digi-connectcore-som.dtsi      | 1 -
 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 1 -
 arch/arm/boot/dts/imx53-smd.dts                        | 1 -
 arch/arm/boot/dts/imx53-tqma53.dtsi                    | 2 --
 arch/arm/boot/dts/imx53-tx53.dtsi                      | 1 -
 arch/arm/boot/dts/imx53-voipac-dmm-668.dtsi            | 1 -
 arch/arm/boot/dts/imx6dl-aristainetos_4.dts            | 1 -
 arch/arm/boot/dts/imx6q-ba16.dtsi                      | 1 -
 arch/arm/boot/dts/imx6q-bx50v3.dtsi                    | 1 -
 arch/arm/boot/dts/imx6q-cm-fx6.dts                     | 1 -
 arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts                | 1 -
 arch/arm/boot/dts/imx6q-evi.dts                        | 3 ---
 arch/arm/boot/dts/imx6q-gw5400-a.dts                   | 1 -
 arch/arm/boot/dts/imx6q-marsboard.dts                  | 1 -
 arch/arm/boot/dts/imx6q-novena.dts                     | 1 -
 arch/arm/boot/dts/imx6qdl-apalis.dtsi                  | 2 --
 arch/arm/boot/dts/imx6qdl-apf6dev.dtsi                 | 1 -
 arch/arm/boot/dts/imx6qdl-aristainetos.dtsi            | 1 -
 arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi           | 3 ---
 arch/arm/boot/dts/imx6qdl-colibri.dtsi                 | 1 -
 arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi           | 1 -
 arch/arm/boot/dts/imx6qdl-gw52xx.dtsi                  | 1 -
 arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi               | 1 -
 arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi           | 1 -
 arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi          | 1 -
 arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi              | 1 -
 arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi           | 1 -
 arch/arm/boot/dts/imx6qdl-rex.dtsi                     | 2 --
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi               | 1 -
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi               | 1 -
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi                 | 1 -
 arch/arm/boot/dts/imx6qdl-ts4900.dtsi                  | 2 --
 arch/arm/boot/dts/imx6qdl-tx6.dtsi                     | 1 -
 arch/arm/boot/dts/imx6sl-evk.dts                       | 1 -
 arch/arm/boot/dts/imx6sx-nitrogen6sx.dts               | 1 -
 arch/arm/boot/dts/imx6ul-tx6ul.dtsi                    | 1 -
 arch/arm/boot/dts/imx7d-sdb.dts                        | 1 -
 47 files changed, 57 deletions(-)

diff --git a/arch/arm/boot/dts/imx1-ads.dts b/arch/arm/boot/dts/imx1-ads.dts
index f504986..5ea28ee 100644
--- a/arch/arm/boot/dts/imx1-ads.dts
+++ b/arch/arm/boot/dts/imx1-ads.dts
@@ -38,7 +38,6 @@
 
 &cspi1 {
 	pinctrl-0 = <&pinctrl_cspi1>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx27-apf27dev.dts b/arch/arm/boot/dts/imx27-apf27dev.dts
index bba3f41..5f84b59 100644
--- a/arch/arm/boot/dts/imx27-apf27dev.dts
+++ b/arch/arm/boot/dts/imx27-apf27dev.dts
@@ -77,7 +77,6 @@
 };
 
 &cspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 28 GPIO_ACTIVE_LOW>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cspi1 &pinctrl_cspi1_cs>;
@@ -95,7 +94,6 @@
 };
 
 &cspi2 {
-	fsl,spi-num-chipselects = <3>;
 	cs-gpios = <&gpio4 21 GPIO_ACTIVE_LOW>,
 		   <&gpio4 27 GPIO_ACTIVE_LOW>,
 		   <&gpio2 17 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts b/arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts
index 27846ff..f565357 100644
--- a/arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts
+++ b/arch/arm/boot/dts/imx27-eukrea-mbimxsd27-baseboard.dts
@@ -81,7 +81,6 @@
 
 &cspi1 {
 	pinctrl-0 = <&pinctrl_cspi1>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 28 GPIO_ACTIVE_LOW>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx27-pdk.dts b/arch/arm/boot/dts/imx27-pdk.dts
index d0ef496..96f442b 100644
--- a/arch/arm/boot/dts/imx27-pdk.dts
+++ b/arch/arm/boot/dts/imx27-pdk.dts
@@ -37,7 +37,6 @@
 &cspi2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cspi2>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 21 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi b/arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi
index 1b62480..4f3e0f4 100644
--- a/arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi
+++ b/arch/arm/boot/dts/imx27-phytec-phycard-s-som.dtsi
@@ -23,7 +23,6 @@
 };
 
 &cspi1 {
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>,
 		   <&gpio4 27 GPIO_ACTIVE_HIGH>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts b/arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts
index cf09e72..2a9198f 100644
--- a/arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts
+++ b/arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts
@@ -69,7 +69,6 @@
 
 &cspi1 {
 	pinctrl-0 = <&pinctrl_cspi1>, <&pinctrl_cspi1cs1>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>,
 		   <&gpio4 27 GPIO_ACTIVE_LOW>;
 };
diff --git a/arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi b/arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi
index b4e955e..82fec93 100644
--- a/arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi
+++ b/arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi
@@ -75,7 +75,6 @@
 &cspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cspi1>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx50-evk.dts b/arch/arm/boot/dts/imx50-evk.dts
index 27d763c..dba2d95 100644
--- a/arch/arm/boot/dts/imx50-evk.dts
+++ b/arch/arm/boot/dts/imx50-evk.dts
@@ -26,7 +26,6 @@
 &cspi {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cspi>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio4 11 0>, <&gpio4 13 0>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts b/arch/arm/boot/dts/imx51-apf51dev.dts
index 0f3fe29..a5e6091 100644
--- a/arch/arm/boot/dts/imx51-apf51dev.dts
+++ b/arch/arm/boot/dts/imx51-apf51dev.dts
@@ -80,7 +80,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio4 24 GPIO_ACTIVE_HIGH>,
 		   <&gpio4 25 GPIO_ACTIVE_HIGH>;
 	status = "okay";
@@ -89,7 +88,6 @@
 &ecspi2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio3 28 GPIO_ACTIVE_LOW>,
 		   <&gpio3 27 GPIO_ACTIVE_LOW>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index f097b4f..873cf24 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -178,7 +178,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio4 24 GPIO_ACTIVE_HIGH>,
 		   <&gpio4 25 GPIO_ACTIVE_LOW>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/imx51-digi-connectcore-som.dtsi b/arch/arm/boot/dts/imx51-digi-connectcore-som.dtsi
index 16fc69c..b821066 100644
--- a/arch/arm/boot/dts/imx51-digi-connectcore-som.dtsi
+++ b/arch/arm/boot/dts/imx51-digi-connectcore-som.dtsi
@@ -24,7 +24,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 24 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
index 7282128..1305b05 100644
--- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
@@ -114,7 +114,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 24 GPIO_ACTIVE_LOW>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx53-smd.dts b/arch/arm/boot/dts/imx53-smd.dts
index 9f51900..472f6f0 100644
--- a/arch/arm/boot/dts/imx53-smd.dts
+++ b/arch/arm/boot/dts/imx53-smd.dts
@@ -63,7 +63,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx53-tqma53.dtsi b/arch/arm/boot/dts/imx53-tqma53.dtsi
index 91a6a9f..85972f2 100644
--- a/arch/arm/boot/dts/imx53-tqma53.dtsi
+++ b/arch/arm/boot/dts/imx53-tqma53.dtsi
@@ -55,7 +55,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <4>;
 	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>,
 		   <&gpio3 24 0>, <&gpio3 25 0>;
 	status = "disabled";
@@ -249,7 +248,6 @@
 &cspi {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cspi>;
-	fsl,spi-num-chipselects = <3>;
 	cs-gpios = <&gpio1 18 0>, <&gpio1 19 0>,
 		   <&gpio1 21 0>;
 	status = "disabled";
diff --git a/arch/arm/boot/dts/imx53-tx53.dtsi b/arch/arm/boot/dts/imx53-tx53.dtsi
index 57e75f1..3a32201 100644
--- a/arch/arm/boot/dts/imx53-tx53.dtsi
+++ b/arch/arm/boot/dts/imx53-tx53.dtsi
@@ -161,7 +161,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <2>;
 	status = "okay";
 
 	cs-gpios = <
diff --git a/arch/arm/boot/dts/imx53-voipac-dmm-668.dtsi b/arch/arm/boot/dts/imx53-voipac-dmm-668.dtsi
index ba689fb..524192c 100644
--- a/arch/arm/boot/dts/imx53-voipac-dmm-668.dtsi
+++ b/arch/arm/boot/dts/imx53-voipac-dmm-668.dtsi
@@ -129,7 +129,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <4>;
 	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>, <&gpio2 16 0>, <&gpio2 17 0>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6dl-aristainetos_4.dts b/arch/arm/boot/dts/imx6dl-aristainetos_4.dts
index d4c4a22..32a812b 100644
--- a/arch/arm/boot/dts/imx6dl-aristainetos_4.dts
+++ b/arch/arm/boot/dts/imx6dl-aristainetos_4.dts
@@ -66,7 +66,6 @@
 };
 
 &ecspi2 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
diff --git a/arch/arm/boot/dts/imx6q-ba16.dtsi b/arch/arm/boot/dts/imx6q-ba16.dtsi
index 308e11c..3577aa7 100644
--- a/arch/arm/boot/dts/imx6q-ba16.dtsi
+++ b/arch/arm/boot/dts/imx6q-ba16.dtsi
@@ -133,7 +133,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6q-bx50v3.dtsi b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
index e4a415f..b1be794 100644
--- a/arch/arm/boot/dts/imx6q-bx50v3.dtsi
+++ b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
@@ -95,7 +95,6 @@
 };
 
 &ecspi5 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi5>;
diff --git a/arch/arm/boot/dts/imx6q-cm-fx6.dts b/arch/arm/boot/dts/imx6q-cm-fx6.dts
index a150bca..ae90d98 100644
--- a/arch/arm/boot/dts/imx6q-cm-fx6.dts
+++ b/arch/arm/boot/dts/imx6q-cm-fx6.dts
@@ -114,7 +114,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>, <&gpio3 19 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
index 908dab6..f28883b 100644
--- a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
+++ b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
@@ -104,7 +104,6 @@
 &ecspi5 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi5>;
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio1 12 0>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 7c7c1a8..fd2220a 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -90,7 +90,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 10 GPIO_ACTIVE_LOW>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
@@ -98,7 +97,6 @@
 };
 
 &ecspi3 {
-	fsl,spi-num-chipselects = <3>;
 	cs-gpios = <&gpio4 24 GPIO_ACTIVE_LOW>,
 		<&gpio4 25 GPIO_ACTIVE_LOW>,
 		<&gpio4 26 GPIO_ACTIVE_LOW>;
@@ -108,7 +106,6 @@
 };
 
 &ecspi5 {
-	fsl,spi-num-chipselects = <4>;
 	cs-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>,
 		<&gpio1 13 GPIO_ACTIVE_LOW>,
 		<&gpio1 12 GPIO_ACTIVE_LOW>,
diff --git a/arch/arm/boot/dts/imx6q-gw5400-a.dts b/arch/arm/boot/dts/imx6q-gw5400-a.dts
index 747bc10..8e84713 100644
--- a/arch/arm/boot/dts/imx6q-gw5400-a.dts
+++ b/arch/arm/boot/dts/imx6q-gw5400-a.dts
@@ -138,7 +138,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6q-marsboard.dts b/arch/arm/boot/dts/imx6q-marsboard.dts
index f7995c5..51220a3 100644
--- a/arch/arm/boot/dts/imx6q-marsboard.dts
+++ b/arch/arm/boot/dts/imx6q-marsboard.dts
@@ -97,7 +97,6 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
 	cs-gpios = <&gpio2 30 GPIO_ACTIVE_LOW>;
-	fsl,spi-num-chipselects = <1>;
 	status = "okay";
 
 	m25p80@0 {
diff --git a/arch/arm/boot/dts/imx6q-novena.dts b/arch/arm/boot/dts/imx6q-novena.dts
index 758bca9..0fa32b2 100644
--- a/arch/arm/boot/dts/imx6q-novena.dts
+++ b/arch/arm/boot/dts/imx6q-novena.dts
@@ -210,7 +210,6 @@
 &ecspi3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi3_novena>;
-	fsl,spi-num-chipselects = <3>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-apalis.dtsi b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
index 8c8a049..82652c1 100644
--- a/arch/arm/boot/dts/imx6qdl-apalis.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apalis.dtsi
@@ -175,7 +175,6 @@
 
 /* Apalis SPI1 */
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio5 25 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
@@ -184,7 +183,6 @@
 
 /* Apalis SPI2 */
 &ecspi2 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio2 26 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
diff --git a/arch/arm/boot/dts/imx6qdl-apf6dev.dtsi b/arch/arm/boot/dts/imx6qdl-apf6dev.dtsi
index 5e7792d..550e100 100644
--- a/arch/arm/boot/dts/imx6qdl-apf6dev.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-apf6dev.dtsi
@@ -176,7 +176,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <3>;
 	cs-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>,
 		   <&gpio4 10 GPIO_ACTIVE_LOW>,
 		   <&gpio4 11 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi b/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi
index 54f4f01..b2debc0 100644
--- a/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-aristainetos.dtsi
@@ -100,7 +100,6 @@
 };
 
 &ecspi4 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 20 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi4>;
diff --git a/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi b/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
index 7fff02c..0d9e96d 100644
--- a/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
@@ -114,7 +114,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <3>;
 	cs-gpios = <&gpio4 9 GPIO_ACTIVE_HIGH
 		    &gpio4 10 GPIO_ACTIVE_HIGH
 		    &gpio4 11 GPIO_ACTIVE_HIGH>;
@@ -124,7 +123,6 @@
 };
 
 &ecspi2 {
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio2 26 GPIO_ACTIVE_HIGH &gpio2 27 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
@@ -132,7 +130,6 @@
 };
 
 &ecspi4 {
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <&gpio3 29 GPIO_ACTIVE_HIGH &gpio5 2 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi4>;
diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index e6faa65..dbad481 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -138,7 +138,6 @@
 
 /* Colibri SSP */
 &ecspi4 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio5 2 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi4>;
diff --git a/arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi b/arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi
index b2c083d..d78312c 100644
--- a/arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-dfi-fs700-m60.dtsi
@@ -29,7 +29,6 @@
 };
 
 &ecspi3 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 24 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi3>;
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index 54aca3a..ab34f15 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -159,7 +159,6 @@
 };
 
 &ecspi3 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 24 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi3>;
diff --git a/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi b/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
index 63acd54..e19f3c3 100644
--- a/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nit6xlite.dtsi
@@ -209,7 +209,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 GPIO_ACTIVE_LOW>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
index 34887a1..e46c914 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi
@@ -353,7 +353,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi
index d80f21a..678ab19 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6_som2.dtsi
@@ -284,7 +284,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index e476d01..c4d28e9 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -255,7 +255,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
index e9801a2..6e5cb6a 100644
--- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi
@@ -76,7 +76,6 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi3>;
 	status = "okay";
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 24 0>;
 
 	flash@0 {
diff --git a/arch/arm/boot/dts/imx6qdl-rex.dtsi b/arch/arm/boot/dts/imx6qdl-rex.dtsi
index 17704a5..5cf90c24 100644
--- a/arch/arm/boot/dts/imx6qdl-rex.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-rex.dtsi
@@ -89,7 +89,6 @@
 };
 
 &ecspi2 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
@@ -97,7 +96,6 @@
 };
 
 &ecspi3 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 26 GPIO_ACTIVE_LOW>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi3>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 52390ba..a2a714d 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -124,7 +124,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1_cs>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index 1f9076e..b0fa0c8 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -241,7 +241,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index 55ef535..63bf95e 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -160,7 +160,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 9 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6qdl-ts4900.dtsi b/arch/arm/boot/dts/imx6qdl-ts4900.dtsi
index 5c26b26..bf17ad5 100644
--- a/arch/arm/boot/dts/imx6qdl-ts4900.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-ts4900.dtsi
@@ -95,7 +95,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
@@ -109,7 +108,6 @@
 };
 
 &ecspi2 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio6 2 GPIO_ACTIVE_HIGH>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
diff --git a/arch/arm/boot/dts/imx6qdl-tx6.dtsi b/arch/arm/boot/dts/imx6qdl-tx6.dtsi
index 2bf2e62..1691714 100644
--- a/arch/arm/boot/dts/imx6qdl-tx6.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tx6.dtsi
@@ -221,7 +221,6 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <
 		&gpio2 30 GPIO_ACTIVE_HIGH
 		&gpio3 19 GPIO_ACTIVE_HIGH
diff --git a/arch/arm/boot/dts/imx6sl-evk.dts b/arch/arm/boot/dts/imx6sl-evk.dts
index be11882..0a90eea 100644
--- a/arch/arm/boot/dts/imx6sl-evk.dts
+++ b/arch/arm/boot/dts/imx6sl-evk.dts
@@ -117,7 +117,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio4 11 0>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6sx-nitrogen6sx.dts b/arch/arm/boot/dts/imx6sx-nitrogen6sx.dts
index 9b817f3..b54b40a 100644
--- a/arch/arm/boot/dts/imx6sx-nitrogen6sx.dts
+++ b/arch/arm/boot/dts/imx6sx-nitrogen6sx.dts
@@ -142,7 +142,6 @@
 };
 
 &ecspi1 {
-	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio2 16 GPIO_ACTIVE_LOW>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
diff --git a/arch/arm/boot/dts/imx6ul-tx6ul.dtsi b/arch/arm/boot/dts/imx6ul-tx6ul.dtsi
index 530e9ca..c784a0b 100644
--- a/arch/arm/boot/dts/imx6ul-tx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul-tx6ul.dtsi
@@ -285,7 +285,6 @@
 &ecspi2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi2>;
-	fsl,spi-num-chipselects = <2>;
 	cs-gpios = <
 		&gpio1 29 GPIO_ACTIVE_HIGH
 		&gpio1 10 GPIO_ACTIVE_HIGH
diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
index 2f33c46..9d15273 100644
--- a/arch/arm/boot/dts/imx7d-sdb.dts
+++ b/arch/arm/boot/dts/imx7d-sdb.dts
@@ -111,7 +111,6 @@
 };
 
 &ecspi3 {
-	fsl,spi-num-chipselects = <1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi3>;
 	cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>;
-- 
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 related

* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2016-12-24  6:01 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
	mark.rutland, netdev, devicetree, linux-kernel, justin,
	Jaret Cantu
In-Reply-To: <1482380314-16440-3-git-send-email-geoff@kuvee.com>

On Wed, Dec 21, 2016 at 11:18:34PM -0500, Geoff Lansberry wrote:
> From: Jaret Cantu <jaret.cantu@timesys.com>
> 
> Repeated polling attempts cause a NULL dereference error to occur.
> This is because the state of the trf7970a is currently reading but
> another request has been made to send a command before it has finished.
> 
> The solution is to properly kill the waiting reading (workqueue)
> before failing on the send.
> 
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---

You've still provided virtually no information on the actual problem(s)
nor justified why you think this is the best solution.  You're adding
code to a section of code that should _never_ be executed so the only
reasonable things I can infer is that there are, at least, two problems:

1) There is a bug causing execution to get into this block of code.

2) Once in this block of code, there is another bug.

You seem to be attempting to fix 2) and completely ignoring 1).
1) is the first bug that needs to be root-caused and fixed.

Also, what exactly is the "NULL dereference error" you mention?
Is this the neard crash you talked about in another thread or is
this a kernel crash?  If it is the kernel crash, please post the
relevant information.  If this is the neard crash - which seems
unlikely - then how can changing a section of kernel code that
shouldn't be executed in the first place fix that?

Mark

^ permalink raw reply

* Re: [PATCH v5 3/6] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Kefeng Wang @ 2016-12-24  6:07 UTC (permalink / raw)
  To: Ding Tianhong, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, oss-fOR+EgIDQEHk1uMJSBkQmQ,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, stuart.yoder-3arQi8VN3Tc,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <1482476669-15596-4-git-send-email-dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>



On 2016/12/23 15:04, Ding Tianhong wrote:
> Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
> potential to contain an erroneous value when the timer value changes".
> Accesses to TVAL (both read and write) are also affected due to the implicit counter
> read.  Accesses to CVAL are not affected.
> 
> The workaround is to reread the system count registers until the value of the second
> read is larger than the first one by less than 32, the system counter can be guaranteed
> not to return wrong value twice by back-to-back read and the error value is always larger
> than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
> 
> The workaround is enabled if the hisilicon,erratum-161601 property is found in
> the timer node in the device tree. This can be overridden with the
> clocksource.arm_arch_timer.hisilicon-161601 boot parameter, which allows KVM
> users to enable the workaround until a mechanism is implemented to
> automatically communicate this information.
> 
> Fix some description for fsl erratum a008585.
> 
> v2: Significant rework based on feedback, including seperate the fsl erratum a008585
>     to another patch, update the erratum name and remove unwanted code.
> 
> v3: Significant rework based on feedback, including fix some alignment problem, make the
>     #define __hisi_161601_read_reg to be private to the .c file instead of being globally
>     visible, add more accurate annotation and modify a bit of logical format to enable
>     arch_timer_read_ool_enabled, remove the kernel commandline parameter
>     clocksource.arm_arch_timer.hisilicon-161601.
> 
> v5: Theoretically the erratum should not occur more than twice in succession when reading
>     the system counter, but it is possible that some interrupts may lead to more than twice
>     read errors, triggering the warning, so setting the number of retries to 50 which is far
>     beyond the number of iterations the loop has been observed to take.
> 
> Signed-off-by: Ding Tianhong <dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/arm64/silicon-errata.txt |  1 +
>  arch/arm64/include/asm/arch_timer.h    |  2 +-
>  drivers/clocksource/Kconfig            |  9 +++++
>  drivers/clocksource/arm_arch_timer.c   | 70 +++++++++++++++++++++++++++++++---
>  4 files changed, 76 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 405da11..1c1a95f 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -63,3 +63,4 @@ stable kernels.
>  | Cavium         | ThunderX SMMUv2 | #27704          | N/A		       |
>  |                |                 |                 |                         |
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
> +| Hisilicon      | Hip0{5,6,7}     | #161601         | HISILICON_ERRATUM_161601|
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index f882c7c..ebf4cde 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -29,7 +29,7 @@
>  
>  #include <clocksource/arm_arch_timer.h>
>  
> -#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585)
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>  extern struct static_key_false arch_timer_read_ool_enabled;
>  #define needs_unstable_timer_counter_workaround() \
>  	static_branch_unlikely(&arch_timer_read_ool_enabled)
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 4866f7a..162d820 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -335,6 +335,15 @@ config FSL_ERRATUM_A008585
>  	  value").  The workaround will only be active if the
>  	  fsl,erratum-a008585 property is found in the timer node.
>  
> +config HISILICON_ERRATUM_161601
> +	bool "Workaround for Hisilicon Erratum 161601"
> +	default y
> +	depends on ARM_ARCH_TIMER && ARM64
> +	help
> +	  This option enables a workaround for Hisilicon Erratum
> +	  161601. The workaround will be active if the hisilicon,erratum-161601
> +	  property is found in the timer node.
> +
>  config ARM_GLOBAL_TIMER
>  	bool "Support for the ARM global timer" if COMPILE_TEST
>  	select CLKSRC_OF if OF
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index e7406ad..9a82496 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -95,15 +95,18 @@ static int __init early_evtstrm_cfg(char *buf)
>   * Architected system timer support.
>   */
>  
> -#ifdef CONFIG_FSL_ERRATUM_A008585
> +#if CONFIG_FSL_ERRATUM_A008585 || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>  struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
>  EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
>  


>  #define        FSL_A008585	0x0001
> +#define        HISILICON_161601	0x0002

It looks like the DEFINE is useless,

> +
> +static struct arch_timer_erratum_workaround arch_timer_hisi_161601 = {
> +	.erratum = HISILICON_161601,

Just a errata description, eg.
	.desc	= "HISILICON ERRATUM 161601"
then....

> +	.read_cntp_tval_el0 = hisi_161601_read_cntp_tval_el0,
> +	.read_cntv_tval_el0 = hisi_161601_read_cntv_tval_el0,
> +	.read_cntvct_el0 = hisi_161601_read_cntvct_el0,
> +};
> +#endif /* CONFIG_HISILICON_ERRATUM_161601 */
[..]

> -#ifdef CONFIG_FSL_ERRATUM_A008585
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>  		/*
>  		 * Don't use the vdso fastpath if errata require using
>  		 * the out-of-line counter accessor.
> @@ -909,10 +960,19 @@ static int __init arch_timer_of_init(struct device_node *np)
>  #ifdef CONFIG_FSL_ERRATUM_A008585
>  	if (!timer_unstable_counter_workaround && of_property_read_bool(np, "fsl,erratum-a008585"))
>  		timer_unstable_counter_workaround = &arch_timer_fsl_a008585;
> +#endif
> +
> +#ifdef CONFIG_HISILICON_ERRATUM_161601
> +	if (!timer_unstable_counter_workaround && of_property_read_bool(np, "hisilicon,erratum-161601"))
> +		timer_unstable_counter_workaround = &arch_timer_hisi_161601;
> +#endif
>  
> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>  	if (timer_unstable_counter_workaround) {
>  		static_branch_enable(&arch_timer_read_ool_enabled);
> -		pr_info("Enabling workaround for FSL erratum A-008585\n");
> +		pr_info("Enabling workaround for %s\n",
> +			timer_unstable_counter_workaround->erratum == FSL_A008585 ?
> +			"FSL ERRATUM A-008585" : "HISILICON ERRATUM 161601");

......>		pr_info("Enabling workaround for %s\n", timer_unstable_counter_workaround->desc)

>  	}
>  #endif
>  
> 

--
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 v5 6/6] arm64: arch_timer: acpi: add hisi timer errata data
From: Kefeng Wang @ 2016-12-24  7:11 UTC (permalink / raw)
  To: Ding Tianhong, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, oss-fOR+EgIDQEHk1uMJSBkQmQ,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, stuart.yoder-3arQi8VN3Tc,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxarm-hv44wF8Li93QT0dZR+AlfA
  Cc: Hanjun Guo
In-Reply-To: <1482476669-15596-7-git-send-email-dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>



On 2016/12/23 15:04, Ding Tianhong wrote:
> From: Hanjun Guo <hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Add hisi timer specific erratum fixes.
> 
> v3: add hisilicon erratum 161601 for ACPI mode.
> 
> v4: update some data structures.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Ding Tianhong <dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/clocksource/arm_arch_timer.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 212bfa5..7b15d2a 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -1089,10 +1089,28 @@ struct gtdt_arch_timer_fixup {
>  	void *context;
>  };
>  
> +#ifdef CONFIG_HISILICON_ERRATUM_161601
> +static void __init erratum_workaround_enable(void *context)
> +{
> +	u64 erratum = (u64) context;
> +
> +	if (erratum & HISILICON_161601) {
> +		timer_unstable_counter_workaround = &arch_timer_hisi_161601;
> +		static_branch_enable(&arch_timer_read_ool_enabled);
> +		pr_info("Enabling workaround for HISILICON ERRATUM 161601\n");
> +	}


Use arch_timer_hisi_161601 for context instead of HISILICON_161601 directly,
we can make erratum_workaround_enable more general.

> +}
> +#endif
> +
>  /* note: this needs to be updated according to the doc of OEM ID
>   * and TABLE ID for different board.
>   */
>  struct gtdt_arch_timer_fixup arch_timer_quirks[] __initdata = {
> +#ifdef CONFIG_HISILICON_ERRATUM_161601
> +	{"HISI", "hip05", 0, &erratum_workaround_enable, (void *) HISILICON_161601},
> +	{"HISI", "hip06", 0, &erratum_workaround_enable, (void *) HISILICON_161601},
> +	{"HISI", "hip07", 0, &erratum_workaround_enable, (void *) HISILICON_161601},
> +#endif





>  };
>  
>  void __init arch_timer_acpi_quirks_handler(char *oem_id,
> 

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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).