* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
From: Pantelis Antoniou @ 2016-11-30 9:07 UTC (permalink / raw)
To: David Gibson
Cc: Frank Rowand, Jon Loeliger, Grant Likely, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161130003900.GC19891@umbus>
Hi David,
> On Nov 30, 2016, at 02:39 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Tue, Nov 29, 2016 at 08:45:16AM -0800, Frank Rowand wrote:
>> On 11/28/16 21:10, David Gibson wrote:
>>> On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
>>>> On 11/28/16 19:10, David Gibson wrote:
>>>>> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
>>>>>> There exists a syntactic sugar version of overlays which
>>>>>> make them simpler to write for the trivial case of a single target.
>>>>
>>>> It also works for multiple targets. (See the example I provided in
>>>> my comment to v10.)
>>>>
>>>>
>>>>>>
>>>>>> Document it in the device tree object internals.
>>>>>>
>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>
>>>>> I'm with Frank that I think this, rather than being regarded mere
>>>>> syntactic sugar, should be considered the primary way of describing
>>>>> overlays.
>>>>>
>>>>> Obviously we need to support the fully written out version as well.
>>>>
>>>> If we need to support the fully written out version, can we make that
>>>> a discouraged, non-preferred method? Maybe require an option to
>>>> enable compiling this style of dts?
>>>
>>> Yeah. To avoid further proliferation of options, I'm thinking a
>>> single "backwards compat" option which would:
>>> - Use the dtb magic instead of dtb magic
>>> - Disable checks which would reject explicit creation of
>>> __overlay__ / __symbols__ / __fixups__ nodes
>>> - Anything other special behaviour we need
>>>
>>>> I can imagine some reasons to support the fully written out version,
>>>> but can we document what those reasons are?
>>>
>>> I believe the main one is the dts files in this format out in the
>>> field. Mind you, I guess we're already requiring them to tweak how
>>> they declare the /plugin/ option.
>>
>> It might be easy to write a program that transforms the expanded
>> format to the simple format. I'll try to make some time to see
>> how difficult it is. The transformation is relatively easy to
>> do manually, but I don't know how many dts files would need to
>> be converted.
>
> It's not totally trivial, because such a program would basically need
> a full dts parser. But.. if we change the dtc internals to work with
> a list of overlays rather than a single tree, there's a relatively
> obvious path to it: we can implement parsing a dtbo input into a set
> of fragments rather than an assembled overlay and dts output in
> fragment form. Then converting from old-dts to dtb and back to dts
> would pretty much do what's needed. At the cost of losing comments,
> though :/.
>
It’s not just the comments. New style dts’es in the kernel now use CPP
pre-processed input which make the dts more readable.
Converting back to dts is not even close to the original source.
You might as well use fdtdump instead.
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
Regards
— Pantelis
^ permalink raw reply
* Re: [PATCH v2 13/13] net: ethernet: ti: cpts: fix overflow check period
From: Richard Cochran @ 2016-11-30 9:12 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, Rob Herring, devicetree, Murali Karicheri,
Wingman Kwok, John Stultz, Thomas Gleixner
In-Reply-To: <20161128230337.6731-14-grygorii.strashko@ti.com>
On Mon, Nov 28, 2016 at 05:03:37PM -0600, Grygorii Strashko wrote:
> The CPTS drivers uses 8sec period for overflow checking with
> assumption that CPTS retclk will not exceed 500MHz. But that's not
> true on some TI platforms (Kesytone 2). As result, it is possible that
> CPTS counter will overflow more than once between two readings.
>
> Hence, fix it by selecting overflow check period dynamically as
> max_sec_before_overflow/2, where
> max_sec_before_overflow = max_counter_val / rftclk_freq.
>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero
From: Andre Przywara @ 2016-11-30 9:25 UTC (permalink / raw)
To: Icenowy Zheng, Alexey Kardashevskiy
Cc: Mark Rutland, devicetree, Vishnu Patekar, Arnd Bergmann,
Jonathan Corbet, linux-doc, Russell King, LKML, Hans de Goede,
Chen-Yu Tsai, Maxime Ripard, linux-arm-kernel
In-Reply-To: <20161129131922.JFbeipav@smtp2o.mail.yandex.net>
Hi,
On 29/11/16 10:19, Icenowy Zheng wrote:
>
> 2016年11月29日 15:16于 Alexey Kardashevskiy <aik@ozlabs.ru>写道：
>>
>>
>>
>> On Wed, Nov 23, 2016 at 6:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> > There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> > Orange Pi Zero, which used a dedicated regulator to power.
>>> >
>>> > Add the device tree node of the regulator, the enable gpio (with
>>> > mmc-pwrseq) and the sdio controller.
>>> >
>>> > There's a out-of-tree driver tested to work with this device tree.
>>
>>
>> btw could you please give a pointer where to find a XR819 driver for
> relatively recent kernel (4.8 may be, just not 3.4)? Thanks.
>
> https://github.com/Icenowy/xradio
I was just curious, so pulled your tree and tried to just compile it. It
still threw warnings at me for ARM, and even more so for arm64.
I fixed all of them and put that on my github[1]. Feel free to just pick
them from there or wait till I manage to clean them up and send you a
pull request.
And also just a a test, I quickly put it in drivers/net/wireless/xradio,
where it compiled fine after registering it with the upper level Kconfig
and Makefile.
And while looking at it: This looks like typical AW code, not even
remotely upstreameable and probably far too complicated. Enabling some
Kconfig options made it complain about missing functions.
Has anyone checked if this is close to an existing WiFi chip? That
wouldn't be a first ;-)
Cheers,
Andre.
[1] https://github.com/apritzel/xradio/commits/quickfixes
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-11-30 9:26 UTC (permalink / raw)
To: Wolfram Sang
Cc: devicetree, alexandre.torgue, linux-kernel, Patrice Chotard,
linux, Rob Herring, linux-i2c, Maxime Coquelin, linux-arm-kernel
In-Reply-To: <20160722070219.GB1605@katana>
Hi Wolfram,
Thanks for reviewing this driver and sorry for this quite long answer.
I was too busy in another project but now I am ready to complete the
upstream of the STM32F4 I2C driver.
2016-07-22 9:02 GMT+02:00 Wolfram Sang <wsa@the-dreams.de>:
> Hi,
>
> thanks for this contribution! Looks mostly good, some comments, though.
>
> On Mon, Jun 20, 2016 at 06:22:48PM +0200, 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 | 863 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 874 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index faa8e68..805acf6 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -873,6 +873,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?
Ok good point. I will fix it in the V4.
>
>> + 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 37f2819..2ac0eb2 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -84,6 +84,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..652d4bf
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-stm32f4.c
>> @@ -0,0 +1,863 @@
>> +/*
>> + * Driver for STMicroelectronics STM32 I2C controller
>> + *
>> + * Copyright (C) M'boumba Cedric Madianga 2015
>> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> + *
>> + * This driver is based on st-i2c.c
>
> i2c-st.c ?
You are right. Thanks.
>
>> + *
>> + * 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))
>> +#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))
>> +#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 2
>> +#define STM32F4_I2C_MAX_FREQ 42
>> +#define FAST_MODE_MAX_RISE_TIME 1000
>> +#define STD_MODE_MAX_RISE_TIME 300
>> +#define MHZ_TO_HZ 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
>> + * @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
>> + */
>> +struct stm32f4_i2c_timings {
>> + u32 rate;
>> + u32 duty;
>> + u32 mul_ccr;
>> + u32 min_ccr;
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_client - 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_client {
>
> I think the name is a little misleading. Check 'struct i2c_client' as a
> comparison. A more comprehensible name might be stm32f4_i2c_msg...
Ok. I will fix it in the V4.
>
>> + u8 addr;
>> + u32 count;
>> + u8 *buf;
>> + int result;
>> + bool stop;
>> +};
>> +
>> +/**
>> + * 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
>> + * @irq_event: interrupt event line for the controller
>> + * @irq_error: interrupt error line for the controller
>> + * @clk: hw i2c clock
>> + * speed: I2C clock frequency of the controller. Standard or Fast only supported
>> + * @client: I2C transfer information
>> + * @rst: I2C reset line
>> + */
>> +struct stm32f4_i2c_dev {
>> + struct i2c_adapter adap;
>> + struct device *dev;
>> + void __iomem *base;
>> + struct completion complete;
>> + int irq_event;
>> + int irq_error;
>> + struct clk *clk;
>> + int speed;
>> + struct stm32f4_i2c_client client;
>> + struct reset_control *rst;
>
> No need to store reset_control. You just use it in probe.
OK. Thanks.
>
>> +};
>> +
>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>> + [STM32F4_I2C_SPEED_STANDARD] = {
>> + .mul_ccr = 1,
>> + .min_ccr = 4,
>> + .duty = 0,
>> + },
>> + [STM32F4_I2C_SPEED_FAST] = {
>> + .mul_ccr = 16,
>> + .min_ccr = 1,
>> + .duty = 1,
>> + },
>> +};
>> +
>> +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;
>> +
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);
>> +}
>> +
>> +static void stm32f4_i2c_disable_it(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;
>> +
>> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> + cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
>> +
>> + clk_rate = clk_get_rate(i2c_dev->clk);
>> + freq = clk_rate / MHZ_TO_HZ;
>> +
>> + if (freq > STM32F4_I2C_MAX_FREQ)
>> + freq = STM32F4_I2C_MAX_FREQ;
>> + if (freq < STM32F4_I2C_MIN_FREQ)
>> + freq = STM32F4_I2C_MIN_FREQ;
>
> clamp() to enforce the range?
Sorry but what do you mean by "clamp()" ?
>
>> +
>> + cr2 |= STM32F4_I2C_CR2_FREQ(freq);
>> + writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
>> +}
>> +
>> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 trise, freq, cr2, val;
>> +
>> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> + freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
>> +
>> + trise = readl_relaxed(i2c_dev->base + STM32F4_I2C_TRISE);
>> + trise &= ~STM32F4_I2C_TRISE_VALUE_MASK;
>> +
>> + /* Maximum rise time computation */
>> + if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
>> + trise |= STM32F4_I2C_TRISE_VALUE((freq + 1));
>> + } else {
>> + val = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
>> + trise |= STM32F4_I2C_TRISE_VALUE((val + 1));
>> + }
>> +
>> + writel_relaxed(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 ccr, clk_rate;
>> + int val;
>> +
>> + ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
>> + ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
>> + STM32F4_I2C_CCR_CCR_MASK);
>> +
>> + clk_rate = clk_get_rate(i2c_dev->clk);
>> + val = clk_rate / MHZ_TO_HZ * 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);
>> +}
>> +
>> +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");
>> + 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_client *c = &i2c_dev->client;
>> +
>> + stm32f4_i2c_write_byte(i2c_dev, *c->buf++);
>> + c->count--;
>> +}
>> +
>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_client *c = &i2c_dev->client;
>> + u32 rbuf;
>> +
>> + rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>> + *c->buf++ = (u8)rbuf & 0xff;
>> + c->count--;
>> +}
>> +
>> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_client *c = &i2c_dev->client;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + stm32f4_i2c_disable_it(i2c_dev);
>> +
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + if (c->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_client *c = &i2c_dev->client;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + if (c->count) {
>> + stm32f4_i2c_write_msg(i2c_dev);
>> + if (!c->count) {
>> + /* Disable BUF interrupt */
>> + 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_client *c = &i2c_dev->client;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + switch (c->count) {
>> + case 1:
>> + stm32f4_i2c_disable_it(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_client *c = &i2c_dev->client;
>> + void __iomem *reg;
>> + u32 mask;
>> + int i;
>> +
>> + switch (c->count) {
>> + case 2:
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + /* Generate STOP or REPSTART */
>> + if (c->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 EVT and ERR interrupt */
>> + 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_client *c = &i2c_dev->client;
>> + void __iomem *reg;
>> + u32 sr2;
>> +
>> + switch (c->count) {
>> + case 0:
>> + stm32f4_i2c_terminate_xfer(i2c_dev);
>> + /* Clear ADDR flag */
>> + sr2 = 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);
>> + sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + if (c->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 PEC Position Ack and clear ADDR flag
>> + */
>> + 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);
>> + sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> +
>> + default:
>> + /* N-byte reception: Enable ACK and clear ADDR flag */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>> + sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> + }
>> +}
>> +
>> +/**
>> + * 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_client *c = &i2c_dev->client;
>> + void __iomem *reg;
>> + u32 real_status, possible_status, ien, sr2;
>> + 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_ITEVTEN) {
>> + possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
>> + if (ien & STM32F4_I2C_CR2_ITBUFEN)
>> + possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
>> + }
>> +
>> + real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>> +
>> + if (!(real_status & possible_status)) {
>> + dev_dbg(i2c_dev->dev,
>> + "spurious evt it (status=0x%08x, ien=0x%08x)\n",
>> + real_status, ien);
>> + return IRQ_NONE;
>> + }
>> +
>> + /* Use __fls() to check error bits first */
>> + flag = __fls(real_status & possible_status);
>> +
>> + switch (1 << flag) {
>> + case STM32F4_I2C_SR1_SB:
>> + stm32f4_i2c_write_byte(i2c_dev, c->addr);
>> + break;
>> +
>> + case STM32F4_I2C_SR1_ADDR:
>> + if (c->addr & I2C_M_RD)
>> + stm32f4_i2c_handle_rx_addr(i2c_dev);
>> + else
>> + sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +
>> + /* Enable ITBUF interrupts */
>> + reg = i2c_dev->base + STM32F4_I2C_CR2;
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> + break;
>> +
>> + case STM32F4_I2C_SR1_BTF:
>> + if (c->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 it unhandled: status=0x%08x)\n", real_status);
>> + return IRQ_NONE;
>> + }
>> +
>> + 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_client *c = &i2c_dev->client;
>> + void __iomem *reg;
>> + u32 real_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;
>> +
>> + real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>> +
>> + if (!(real_status & possible_status)) {
>> + dev_dbg(i2c_dev->dev,
>> + "spurious err it (status=0x%08x, ien=0x%08x)\n",
>> + real_status, ien);
>> + return IRQ_NONE;
>> + }
>> +
>> + /* Use __fls() to check error bits first */
>> + flag = __fls(real_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);
>> + c->result = -EIO;
>> + break;
>> +
>> + case STM32F4_I2C_SR1_ARLO:
>> + reg = i2c_dev->base + STM32F4_I2C_SR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
>> + c->result = -EAGAIN;
>> + break;
>> +
>> + case STM32F4_I2C_SR1_AF:
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> + c->result = -EIO;
>> + break;
>> +
>> + default:
>> + dev_err(i2c_dev->dev,
>> + "err it unhandled: status=0x%08x)\n", real_status);
>> + return IRQ_NONE;
>> + }
>> +
>> + stm32f4_i2c_soft_reset(i2c_dev);
>> + stm32f4_i2c_disable_it(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_client *c = &i2c_dev->client;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + unsigned long timeout;
>> + u32 mask;
>> + int ret;
>> +
>> + c->addr = (u8)(msg->addr << 1);
>> + c->addr |= (msg->flags & I2C_M_RD);
>
> Use the shiny new "i2c_8bit_addr_from_msg()" for the last two lines.
Good. Thanks. I will use it in the V4.
>
>> + c->buf = msg->buf;
>> + c->count = msg->len;
>> + c->result = 0;
>> + c->stop = is_last;
>> +
>> + reinit_completion(&i2c_dev->complete);
>> +
>> + /* Enable ITEVT and ITERR 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 = c->result;
>> +
>> + /* Disable PEC position Ack */
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_POS);
>> +
>> + if (!timeout) {
>> + dev_err(i2c_dev->dev, "Access to slave 0x%x timed out\n",
>> + c->addr >> 1);
>
> I don't recommend writing err messages for timeouts. They regularly
> happen, e.g. when an eeprom is doing a page write cycle.
OK
>
>> + 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 : i;
>> +}
>> +
>> +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 clk_rate;
>> + struct i2c_adapter *adap;
>> + 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);
>> +
>> + i2c_dev->irq_event = irq_of_parse_and_map(np, 0);
>> + if (!i2c_dev->irq_event) {
>> + dev_err(&pdev->dev, "IRQ missing or invalid\n");
>> + return -EINVAL;
>> + }
>> +
>> + i2c_dev->irq_error = irq_of_parse_and_map(np, 1);
>> + if (!i2c_dev->irq_error) {
>> + dev_err(&pdev->dev, "IRQ 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;
>> + }
>> +
>> + i2c_dev->rst = devm_reset_control_get(&pdev->dev, NULL);
>> + if (IS_ERR(i2c_dev->rst)) {
>> + dev_err(&pdev->dev, "Error: Missing controller reset\n");
>> + return PTR_ERR(i2c_dev->rst);
>> + }
>> + reset_control_assert(i2c_dev->rst);
>> + udelay(2);
>> + reset_control_deassert(i2c_dev->rst);
>> +
>> + i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
>> + ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
>> + if ((!ret) && (clk_rate == 400000))
>> + i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
>> +
>> + i2c_dev->dev = &pdev->dev;
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_event,
>> + NULL, stm32f4_i2c_isr_event,
>> + IRQF_ONESHOT, pdev->name, i2c_dev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to request irq %i\n",
>> + i2c_dev->irq_error);
>> + goto clk_free;
>> + }
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_error,
>> + NULL, stm32f4_i2c_isr_error,
>> + IRQF_ONESHOT, pdev->name, i2c_dev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to request irq %i\n",
>> + i2c_dev->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) {
>> + dev_err(&pdev->dev, "Failed to add adapter\n");
>
> Please drop this messages. We have patches in i2c/for-next where the
> core will report all errors.
OK
>
>> + goto clk_free;
>> + }
>> +
>> + platform_set_drvdata(pdev, i2c_dev);
>> +
>> + dev_info(i2c_dev->dev, "STM32F4 I2C driver initialized\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
>>
^ permalink raw reply
* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Jean-Francois Moine @ 2016-11-30 9:27 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4614815.L3DQhhBy6d@avalon>
On Wed, 30 Nov 2016 10:20:21 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> > Well, I don't see what this connector can be.
> > May you give me a DT example?
>
> Sure.
>
> arch/arm/boot/dts/r8a7791-koelsch.dts
>
> /* HDMI encoder */
>
> hdmi@39 {
> compatible = "adi,adv7511w";
> reg = <0x39>;
> interrupt-parent = <&gpio3>;
> interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
>
> adi,input-depth = <8>;
> adi,input-colorspace = "rgb";
> adi,input-clock = "1x";
> adi,input-style = <1>;
> adi,input-justification = "evenly";
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> adv7511_in: endpoint {
> remote-endpoint = <&du_out_rgb>;
> };
> };
>
> port@1 {
> reg = <1>;
> adv7511_out: endpoint {
> remote-endpoint = <&hdmi_con>;
> };
> };
> };
> };
>
> /* HDMI connector */
>
> hdmi-out {
> compatible = "hdmi-connector";
> type = "a";
>
> port {
> hdmi_con: endpoint {
> remote-endpoint = <&adv7511_out>;
> };
> };
> };
Hi Laurent,
Sorry for I don't see the interest:
- it is obvious that the HDMI connector is a 'hdmi-connector'!
- the physical connector type may be changed on any board by a soldering
iron or a connector to connector cable.
- what does the software do with the connector type?
- why not to put the connector information in the HDMI device?
And, if I follow you, the graph of ports could also be used to describe
the way the various parts of the SoCs are powered, to describe the pin
connections, to describe the USB connectors, to describe the board
internal hubs and bridges...
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: Overlays and boolean properties
From: Phil Elwell @ 2016-11-30 9:29 UTC (permalink / raw)
To: David Gibson, Pantelis Antoniou
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Devicetree Compiler
In-Reply-To: <20161130034733.GH19891@umbus>
Hi David,
On 30/11/2016 03:47, David Gibson wrote:
> On Tue, Nov 29, 2016 at 03:10:40PM +0200, Pantelis Antoniou wrote:
>> Hi Phil,
>>
>>> On Nov 29, 2016, at 15:06 , Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org> wrote:
>>>
>>> Boolean properties are defined as being properties with no content, that
>>> are true if present and false if absent. They pose a problem for DT
>>> overlays, since the proposed (and widely used) overlay mechanism does
>>> not allow for properties (or nodes) to be deleted; overlays can only
>>> make a false property true, so boolean properties are effectively
>>> monostable - once true they become immutable.
>>>
>>> The standard DT syntax includes /delete-property/ and /delete-node/
>>> directives that do what you would expect from their names, but that
>>> facility is not available to overlays. There is no FDT node that
>>> represents the deletion - the directives are acted on immediately
> Uh.. only partially true. They're acted on during the compile run,
> but not during the parse. dtc does have an internal representation of
> node or property deletions that gets resolved when we combine the
> fragments in the source file.
>>> - so
>>> we would need some extra markup - say __delete_property__ and
>>> __delete_node__ - to hold the names of items to be deleted.
> So, in principle this wouldn't be that hard - we'd just need to
> translate dtc's internal representation into a representation in the
> dtb. That could be done with special properties, or with new opcodes
> at the dtb encoding level.
>
>>> Before I take this further, does anybody have any thoughts on the idea?
> So.. the first question, is do we have a pressing use case for this?
> dtbos can (apart from this) alter anything in a base tree, but doing
> so isn't often a good idea. Usually they'll just add new nodes and
> properties.
This is more of a real world example than a pressing use case, since the
number of people affected is small and there is workaround, albeit an
ugly one.
The Raspberry Pi SoCs have two SD interfaces - one SDIO-capable and one
not. On the Pi3B the SDIO-capable interface is used to drive the WiFi
interface. This WiFi interface doesn't like being continuously prodded
to see if it is still there, so the base DTB sets the "non-removable"
boolean property. Not all Pi3 users want WiFi, preferring instead to
wire up an MMC device or second SD slot via the expansion header. A DT
overlay allows them to modify the pin functions to achieve this, but the
overlay can't (easily) remove the aptly-named "non-removable". The ugly
workaround is to disable the original MMC node and create a clone with a
different name and without the unwanted property.
>> The original patchset did support removing properties (by prefixing them with -).
>>
>> I can revive that if we have consensus about the format/method.
> On the whole, I'd prefer not to see extensions of the existing overlay
> format - instead I'd like to see focus on a new and better thought out
> connector format.
Will this connector mechanism allow a populated DT to be modified, or
would you always have to start with an "empty" board and only add that
which you want? I like the flexibility to be able to perform arbitrary
DT modifications (except deletions) - it has been very useful for
testing and performing staggered rollouts of new functionality.
Phil
^ permalink raw reply
* Re: [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Jean-Francois Moine @ 2016-11-30 9:35 UTC (permalink / raw)
To: Maxime Ripard
Cc: Icenowy Zheng, Chen-Yu Tsai, Jernej Skrabec,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161129215932.kmk6qodthicadpyb@lukather>
On Tue, 29 Nov 2016 22:59:32 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > > I'm still not sure which pipeline should I use.
> > >
> > > And, it seems that HDMI Slow Clock is not needed?
> > >
> > > (seems that it's only for EDID, but simplefb won't use EDID)
> >
> > So, I don't see how this may work.
> > How can the u-boot know the resolutions of the HDMI display device?
> >
> > In other words: I have a new H3 board with the last u-boot and kernel.
> > I plug my (rather old or brand new) HDMI display device.
> > After powering on the system, I hope to get something on the screen.
> > How?
>
> If it works like the driver for the first display engine in U-Boot, it
> will use the preferred mode reported by the EDID, and will fallback to
> 1024x768 if it cannot access it.
Icenowy wrote: "simplefb won't use EDID"
Then, if it is like in the kernel, the 1024x768 mode is VGA. It does
not work with HDMI (different timings).
> Maybe it would be worth exchanging on the EDID code that has been done
> for the u-boot driver too, so that it can be fixed in your driver.
The u-boot got my code, and, up to now, I could not fix the random or
permanent failures of EDID reading in some boards.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v2 0/5] Add support for the Armada 3700 SPI controller
From: Romain Perier @ 2016-11-30 9:43 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).
This set of patches adds a basic support for the CPIO mode, then it
enables the FIFO mode (CPU-side only, DMA not supported yet). It also
adds the required definitions of the spi nodes to the devicetree.
Romain Perier (5):
spi: Add basic support for Armada 3700 SPI Controller
spi: armada-3700: Add support for the FIFO mode
dt-bindings: spi: Add documentation for the Armada 3700 SPI Controller
arm64: dts: marvell: Add definition of SPI controller for Armada 3700
arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
.../devicetree/bindings/spi/spi-armada-3700.txt | 25 +
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 +
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 13 +
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-armada-3700.c | 1040 ++++++++++++++++++++
6 files changed, 1116 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
create mode 100644 drivers/spi/spi-armada-3700.c
--
2.9.3
--
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
* [PATCH v2 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-11-30 9:43 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects, supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).
This commit adds basic driver support for CPIO mode and single SPI
transfer. In this mode, the CPU asserts cs, outputs or inputs data from
the current SPI device. Data transfers are copied by 1 or 4 bytes using
the SPI registers.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-armada-3700.c | 651 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 659 insertions(+)
create mode 100644 drivers/spi/spi-armada-3700.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..6ade1ca 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
This enables support for the SPI controller present on the
Atheros AR71XX/AR724X/AR913X SoCs.
+config SPI_ARMADA_3700
+ tristate "Marvell Armada 3700 SPI Controller"
+ depends on ARCH_MVEBU && OF
+ help
+ This enables support for the SPI controller present on the
+ Marvell Armada 3700 SoCs.
+
config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_ALTERA) += spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700) += spi-armada-3700.o
obj-$(CONFIG_SPI_ATMEL) += spi-atmel.o
obj-$(CONFIG_SPI_ATH79) += spi-ath79.o
obj-$(CONFIG_SPI_AU1550) += spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..cc9e1b2
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,651 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
+ * Author: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME "armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT 10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG 0x00
+#define A3700_SPI_IF_CFG_REG 0x04
+#define A3700_SPI_DATA_OUT_REG 0x08
+#define A3700_SPI_DATA_IN_REG 0x0C
+#define A3700_SPI_IF_INST_REG 0x10
+#define A3700_SPI_IF_ADDR_REG 0x14
+#define A3700_SPI_IF_RMODE_REG 0x18
+#define A3700_SPI_IF_HDR_CNT_REG 0x1C
+#define A3700_SPI_IF_DIN_CNT_REG 0x20
+#define A3700_SPI_IF_TIME_REG 0x24
+#define A3700_SPI_INT_STAT_REG 0x28
+#define A3700_SPI_INT_MASK_REG 0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW BIT(8)
+#define A3700_SPI_WFIFO_FULL BIT(7)
+#define A3700_SPI_WFIFO_EMPTY BIT(6)
+#define A3700_SPI_RFIFO_FULL BIT(5)
+#define A3700_SPI_RFIFO_EMPTY BIT(4)
+#define A3700_SPI_WFIFO_RDY BIT(3)
+#define A3700_SPI_RFIFO_RDY BIT(2)
+#define A3700_SPI_XFER_RDY BIT(1)
+#define A3700_SPI_XFER_DONE BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS BIT(28)
+#define A3700_SPI_RFIFO_THRS BIT(24)
+#define A3700_SPI_AUTO_CS BIT(20)
+#define A3700_SPI_DMA_RD_EN BIT(18)
+#define A3700_SPI_FIFO_MODE BIT(17)
+#define A3700_SPI_SRST BIT(16)
+#define A3700_SPI_XFER_START BIT(15)
+#define A3700_SPI_XFER_STOP BIT(14)
+#define A3700_SPI_INST_PIN BIT(13)
+#define A3700_SPI_ADDR_PIN BIT(12)
+#define A3700_SPI_DATA_PIN1 BIT(11)
+#define A3700_SPI_DATA_PIN0 BIT(10)
+#define A3700_SPI_FIFO_FLUSH BIT(9)
+#define A3700_SPI_RW_EN BIT(8)
+#define A3700_SPI_CLK_POL BIT(7)
+#define A3700_SPI_CLK_PHA BIT(6)
+#define A3700_SPI_BYTE_LEN BIT(5)
+#define A3700_SPI_CLK_PRESCALE BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK (0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT 28
+#define A3700_SPI_RFIFO_THRS_BIT 24
+#define A3700_SPI_FIFO_THRS_MASK 0x7
+
+#define A3700_SPI_DATA_PIN_MASK 0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT 12
+#define A3700_SPI_DUMMY_CNT_MASK 0x7
+#define A3700_SPI_RMODE_CNT_BIT 8
+#define A3700_SPI_RMODE_CNT_MASK 0x3
+#define A3700_SPI_ADDR_CNT_BIT 4
+#define A3700_SPI_ADDR_CNT_MASK 0x7
+#define A3700_SPI_INSTR_CNT_BIT 0
+#define A3700_SPI_INSTR_CNT_MASK 0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
+
+struct a3700_spi {
+ struct spi_master *master;
+ void __iomem *base;
+ struct clk *clk;
+ unsigned int irq;
+ unsigned int flags;
+ bool last_xfer;
+ const u8 *tx_buf;
+ u8 *rx_buf;
+ size_t buf_len;
+ u8 byte_len;
+ u32 wait_mask;
+ struct completion done;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+ return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+ writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_AUTO_CS;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ val |= (A3700_SPI_EN << cs);
+ spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+ unsigned int cs)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ val &= ~(A3700_SPI_EN << cs);
+ spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+ unsigned int pin_mode)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+ val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+ switch (pin_mode) {
+ case 1:
+ break;
+ case 2:
+ val |= A3700_SPI_DATA_PIN0;
+ break;
+ case 4:
+ val |= A3700_SPI_DATA_PIN1;
+ break;
+ default:
+ dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+ return -EINVAL;
+ }
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ return 0;
+}
+
+static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_FIFO_MODE;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+ unsigned int mode_bits)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+ if (mode_bits & SPI_CPOL)
+ val |= A3700_SPI_CLK_POL;
+ else
+ val &= ~A3700_SPI_CLK_POL;
+
+ if (mode_bits & SPI_CPHA)
+ val |= A3700_SPI_CLK_PHA;
+ else
+ val &= ~A3700_SPI_CLK_PHA;
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+ unsigned int speed_hz, u16 mode)
+{
+ u32 val;
+ u32 prescale;
+
+ prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+ val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ if (prescale <= 2) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+ val |= A3700_SPI_CLK_CAPT_EDGE;
+ spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+ }
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+ if (mode & SPI_CPOL)
+ val |= A3700_SPI_CLK_POL;
+
+ if (mode & SPI_CPHA)
+ val |= A3700_SPI_CLK_PHA;
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (len == 4)
+ val |= A3700_SPI_BYTE_LEN;
+ else
+ val &= ~A3700_SPI_BYTE_LEN;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+ struct spi_master *master = a3700_spi->master;
+ u32 val;
+ int i;
+
+ /* Reset SPI unit */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_SRST;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ for (i = 0; i < A3700_SPI_TIMEOUT; i++)
+ udelay(1);
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_SRST;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ /* Disable AUTO_CS and deactivate all chip-selects */
+ a3700_spi_auto_cs_unset(a3700_spi);
+ for (i = 0; i < master->num_chipselect; i++)
+ a3700_spi_deactivate_cs(a3700_spi, i);
+
+ a3700_spi_pin_mode_set(a3700_spi, 0);
+
+ /* Be sure that FIFO mode is disabled */
+ a3700_spi_fifo_mode_unset(a3700_spi);
+
+ /* Set SPI mode */
+ a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+ /* Reset counters */
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+ /* Mask the interrupts and clear cause bits */
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+ return 0;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+ struct spi_master *master = dev_id;
+ struct a3700_spi *a3700_spi;
+ u32 cause;
+
+ a3700_spi = spi_master_get_devdata(master);
+
+ /* Get interrupt causes */
+ cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+ /* mask and acknowledge the SPI interrupts */
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+ /* Wake up the transfer */
+ if (a3700_spi->wait_mask & cause)
+ complete(&a3700_spi->done);
+
+ return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+ struct a3700_spi *a3700_spi;
+ unsigned int timeout;
+ unsigned int ctrl_reg;
+ unsigned long timeout_jiffies;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+
+ /* SPI interrupt is edge-triggered, which means an interrupt will
+ * be generated only when detecting a specific status bit changed
+ * from '0' to '1'. So when we start waiting for a interrupt, we
+ * need to check status bit in control reg first, if it is already 1,
+ * then we do not need to wait for interrupt
+ */
+ ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ if (a3700_spi->wait_mask & ctrl_reg)
+ return true;
+
+ reinit_completion(&a3700_spi->done);
+
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+ a3700_spi->wait_mask);
+
+ timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+ timeout = wait_for_completion_timeout(&a3700_spi->done,
+ timeout_jiffies);
+
+ a3700_spi->wait_mask = 0;
+
+ if (timeout)
+ return true;
+
+ /* there might be the case that right after we checked the
+ * status bits in this routine and before start to wait for
+ * interrupt by wait_for_completion_timeout, the interrupt
+ * happens, to avoid missing it we need to double check
+ * status bits in control reg, if it is already 1, then
+ * consider that we have the interrupt successfully and
+ * return true.
+ */
+ ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ if (a3700_spi->wait_mask & ctrl_reg)
+ return true;
+
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+ return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+ unsigned int bit_mask)
+{
+ struct a3700_spi *a3700_spi;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+ a3700_spi->wait_mask = bit_mask;
+
+ return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+
+ a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+}
+
+static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
+{
+ u32 val, data;
+
+ if (a3700_spi->buf_len % a3700_spi->byte_len)
+ return -EINVAL;
+
+ /* Read bytes from data in register */
+ val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+
+ if (a3700_spi->byte_len == 4)
+ data = be32_to_cpu(val);
+ else
+ data = val;
+
+ memcpy(a3700_spi->rx_buf, &data, a3700_spi->byte_len);
+
+ a3700_spi->buf_len -= a3700_spi->byte_len;
+ a3700_spi->rx_buf += a3700_spi->byte_len;
+
+ /* Request next 1 or 4 bytes data */
+ if (a3700_spi->buf_len)
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+ return 0;
+}
+
+static int a3700_spi_write_data(struct a3700_spi *a3700_spi)
+{
+ u32 val = 0;
+
+ if (a3700_spi->buf_len % a3700_spi->byte_len)
+ return -EINVAL;
+
+ /* Write bytes from data out register */
+ if (a3700_spi->byte_len == 4)
+ val = cpu_to_be32(*(u32 *)a3700_spi->tx_buf);
+ else
+ val = a3700_spi->tx_buf[0];
+
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+ a3700_spi->buf_len -= a3700_spi->byte_len;
+ a3700_spi->tx_buf += a3700_spi->byte_len;
+
+ return 0;
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+ if (!enable)
+ a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+ else
+ a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ struct spi_device *spi = message->spi;
+
+ a3700_spi_bytelen_set(a3700_spi, 1);
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret = 0;
+
+ a3700_spi_transfer_setup(spi, xfer);
+
+ a3700_spi->tx_buf = xfer->tx_buf;
+ a3700_spi->rx_buf = xfer->rx_buf;
+ a3700_spi->buf_len = xfer->len;
+
+ /* Start READ transfer by writing dummy data to DOUT register */
+ if (xfer->rx_buf)
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+ while (a3700_spi->buf_len) {
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto err;
+ }
+
+ if (a3700_spi->tx_buf) {
+ ret = a3700_spi_write_data(a3700_spi);
+ if (ret)
+ goto err;
+ }
+
+ if (a3700_spi->rx_buf) {
+ ret = a3700_spi_read_data(a3700_spi);
+ if (ret)
+ goto err;
+ }
+ }
+
+err:
+ spi_finalize_current_transfer(master);
+ return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct spi_device *spi = message->spi;
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+ { .compatible = "marvell,armada-3700-spi", .data = NULL },
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_of_match_table);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *of_node = dev->of_node;
+ struct resource *res;
+ struct spi_master *master;
+ struct a3700_spi *spi;
+ u32 num_cs = 0;
+ int ret = 0;
+
+ master = spi_alloc_master(dev, sizeof(*spi));
+ if (!master) {
+ dev_err(dev, "master allocation failed\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+ dev_err(dev, "could not find num-cs\n");
+ ret = -ENXIO;
+ goto error;
+ }
+
+ master->bus_num = (pdev->id != -1) ? pdev->id : 0;
+ master->dev.of_node = of_node;
+ master->mode_bits = SPI_MODE_3;
+ master->num_chipselect = num_cs;
+ master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+ master->prepare_message = a3700_spi_prepare_message;
+ master->transfer_one = a3700_spi_transfer_one;
+ master->unprepare_message = a3700_spi_unprepare_message;
+ master->set_cs = a3700_spi_set_cs;
+
+ platform_set_drvdata(pdev, master);
+
+ spi = spi_master_get_devdata(master);
+ memset(spi, 0, sizeof(struct a3700_spi));
+
+ spi->master = master;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ spi->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(spi->base)) {
+ ret = PTR_ERR(spi->base);
+ goto error;
+ }
+
+ spi->irq = platform_get_irq(pdev, 0);
+ if (spi->irq < 0) {
+ dev_err(dev, "could not get irq: %d\n", spi->irq);
+ ret = -ENXIO;
+ goto error;
+ }
+
+ init_completion(&spi->done);
+
+ spi->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(spi->clk)) {
+ dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+ goto error;
+ }
+
+ ret = clk_prepare_enable(spi->clk);
+ if (ret) {
+ dev_err(dev, "could not prepare clk: %d\n", ret);
+ goto error;
+ }
+
+ ret = a3700_spi_init(spi);
+ if (ret)
+ goto error_clk;
+
+ ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+ dev_name(dev), master);
+ if (ret) {
+ dev_err(dev, "could not request IRQ: %d\n", ret);
+ goto error_clk;
+ }
+
+ ret = devm_spi_register_master(dev, master);
+ if (ret) {
+ dev_err(dev, "Failed to register master\n");
+ goto error_clk;
+ }
+
+ dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
+ (unsigned long)res->start, spi->irq);
+
+ return 0;
+
+error_clk:
+ clk_disable_unprepare(spi->clk);
+error:
+ spi_master_put(master);
+out:
+ return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct a3700_spi *spi = spi_master_get_devdata(master);
+
+ clk_disable_unprepare(spi->clk);
+ spi_master_put(master);
+
+ return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(a3700_spi_dt_ids),
+ },
+ .probe = a3700_spi_probe,
+ .remove = a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 v2 2/5] spi: armada-3700: Add support for the FIFO mode
From: Romain Perier @ 2016-11-30 9:43 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
In FIFO mode, dedicated registers are used to store the instruction,
the address, the read mode and the data. Write and Read FIFO are used
to store the outcoming or incoming data. The CPU no longer has to assert
each byte. The data FIFOs are accessible via DMA or by the CPU.
This commit adds support for the FIFO mode with the CPU.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v2:
- Removed a3700_spi_bytelen_set from the setup function, it was accidentally
let here and not required, as it is configured in the prepare callback now
(defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
detection with jedec.
drivers/spi/spi-armada-3700.c | 409 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 399 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index cc9e1b2..72f1818 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -99,19 +99,28 @@
/* A3700_SPI_IF_TIME_REG */
#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
+/* Flags and macros for struct a3700_spi */
+#define HAS_FIFO BIT(0)
+#define A3700_INSTR_CNT 1
+#define A3700_ADDR_CNT 3
+#define A3700_DUMMY_CNT 1
+
struct a3700_spi {
struct spi_master *master;
void __iomem *base;
struct clk *clk;
unsigned int irq;
unsigned int flags;
- bool last_xfer;
+ bool xmit_data;
const u8 *tx_buf;
u8 *rx_buf;
size_t buf_len;
u8 byte_len;
u32 wait_mask;
struct completion done;
+ u32 addr_cnt;
+ u32 instr_cnt;
+ size_t hdr_cnt;
};
static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -180,12 +189,15 @@ static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
return 0;
}
-static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
{
u32 val;
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
- val &= ~A3700_SPI_FIFO_MODE;
+ if (a3700_spi->flags & HAS_FIFO)
+ val |= A3700_SPI_FIFO_MODE;
+ else
+ val &= ~A3700_SPI_FIFO_MODE;
spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
}
@@ -255,11 +267,30 @@ static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
a3700_spi->byte_len = len;
}
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+ int timeout = A3700_SPI_TIMEOUT;
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_FIFO_FLUSH;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_FIFO_FLUSH))
+ return 0;
+ udelay(1);
+ }
+
+ return -ETIMEDOUT;
+}
+
static int a3700_spi_init(struct a3700_spi *a3700_spi)
{
struct spi_master *master = a3700_spi->master;
u32 val;
- int i;
+ int i, ret = 0;
/* Reset SPI unit */
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -278,10 +309,8 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
for (i = 0; i < master->num_chipselect; i++)
a3700_spi_deactivate_cs(a3700_spi, i);
- a3700_spi_pin_mode_set(a3700_spi, 0);
-
- /* Be sure that FIFO mode is disabled */
- a3700_spi_fifo_mode_unset(a3700_spi);
+ /* Enable FIFO mode */
+ a3700_spi_fifo_mode_set(a3700_spi);
/* Set SPI mode */
a3700_spi_mode_set(a3700_spi, master->mode_bits);
@@ -294,7 +323,7 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
- return 0;
+ return ret;
}
static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
@@ -380,14 +409,34 @@ static bool a3700_spi_transfer_wait(struct spi_device *spi,
return a3700_spi_wait_completion(spi);
}
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+ unsigned int bytes)
+{
+ u32 val;
+
+ if (a3700_spi->flags & HAS_FIFO) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+ val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+ val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+ val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ }
+}
+
static void a3700_spi_transfer_setup(struct spi_device *spi,
struct spi_transfer *xfer)
{
struct a3700_spi *a3700_spi;
+ unsigned int byte_len;
a3700_spi = spi_master_get_devdata(spi->master);
a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+ byte_len = xfer->bits_per_word >> 3;
+
+ a3700_spi_fifo_thres_set(a3700_spi, byte_len);
}
static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
@@ -447,6 +496,168 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
}
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+ u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+ u32 val = 0;
+
+ /* Clear the header registers */
+ spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+ /* Set header counters */
+ if (a3700_spi->tx_buf) {
+ if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+ instr_cnt = a3700_spi->buf_len;
+ } else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+ a3700_spi->addr_cnt)) {
+ instr_cnt = a3700_spi->instr_cnt;
+ addr_cnt = a3700_spi->buf_len - instr_cnt;
+ } else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+ instr_cnt = a3700_spi->instr_cnt;
+ addr_cnt = a3700_spi->addr_cnt;
+ /* Need to handle the normal write case with 1 byte
+ * data
+ */
+ if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+ dummy_cnt = a3700_spi->buf_len - instr_cnt -
+ addr_cnt;
+ }
+ val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+ << A3700_SPI_INSTR_CNT_BIT);
+ val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+ << A3700_SPI_ADDR_CNT_BIT);
+ val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+ << A3700_SPI_DUMMY_CNT_BIT);
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+ /* Update the buffer length to be transferred */
+ a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+ /* Set Instruction */
+ val = 0;
+ while (instr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+ /* Set Address */
+ val = 0;
+ while (addr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+ int i = 0;
+
+ while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+ val = 0;
+ if (a3700_spi->buf_len >= 4) {
+ val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+ a3700_spi->buf_len -= 4;
+ a3700_spi->tx_buf += 4;
+ } else {
+ /*
+ * If the remained buffer length is less than 4-bytes,
+ * we should pad the write buffer with all ones. So that
+ * it avoids overwrite the unexpected bytes following
+ * the last one.
+ */
+ val = GENMASK(31, 0);
+ while (a3700_spi->buf_len) {
+ val &= ~(0xff << (8 * i));
+ val |= *a3700_spi->tx_buf++ << (8 * i);
+ i++;
+ a3700_spi->buf_len--;
+
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+ val);
+ }
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+ u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+ return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+ val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+ if (a3700_spi->buf_len >= 4) {
+ u32 data = le32_to_cpu(val);
+ memcpy(a3700_spi->rx_buf, &data, 4);
+
+ a3700_spi->buf_len -= 4;
+ a3700_spi->rx_buf += 4;
+ } else {
+ /*
+ * When remain bytes is not larger than 4, we should
+ * avoid memory overwriting and just write the left rx
+ * buffer bytes.
+ */
+ while (a3700_spi->buf_len) {
+ *a3700_spi->rx_buf = val & 0xff;
+ val >>= 8;
+
+ a3700_spi->buf_len--;
+ a3700_spi->rx_buf++;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+ int timeout = A3700_SPI_TIMEOUT;
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_XFER_START))
+ break;
+ udelay(1);
+ }
+
+ a3700_spi_fifo_flush(a3700_spi);
+
+ val &= ~A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
static int a3700_spi_prepare_message(struct spi_master *master,
struct spi_message *message)
{
@@ -463,12 +674,28 @@ static int a3700_spi_prepare_message(struct spi_master *master,
return 0;
}
+static int a3700_spi_prepare_fifo_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret;
+
+ /* Flush the FIFOs */
+ ret = a3700_spi_fifo_flush(a3700_spi);
+ if (ret)
+ return ret;
+
+ a3700_spi_bytelen_set(a3700_spi, 4);
+
+ return 0;
+}
+
static int a3700_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *xfer)
{
struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
- int ret = 0;
+ int ret;
a3700_spi_transfer_setup(spi, xfer);
@@ -505,6 +732,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
return ret;
}
+static int a3700_spi_fifo_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret = 0, timeout = A3700_SPI_TIMEOUT;
+ unsigned int nbits = 0;
+ u32 val;
+
+ a3700_spi_transfer_setup(spi, xfer);
+
+ a3700_spi->tx_buf = xfer->tx_buf;
+ a3700_spi->rx_buf = xfer->rx_buf;
+ a3700_spi->buf_len = xfer->len;
+
+ /* SPI transfer headers */
+ a3700_spi_header_set(a3700_spi);
+
+ if (xfer->tx_buf)
+ nbits = xfer->tx_nbits;
+ else if (xfer->rx_buf)
+ nbits = xfer->rx_nbits;
+
+ a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+ if (xfer->rx_buf) {
+ /* Set read data length */
+ spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+ a3700_spi->buf_len);
+ /* Start READ transfer */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_RW_EN;
+ val |= A3700_SPI_XFER_START;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ } else if (xfer->tx_buf) {
+ /* Start Write transfer */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ /*
+ * If there are data to be written to the SPI device, xmit_data
+ * flag is set true; otherwise the instruction in SPI_INSTR does
+ * not require data to be written to the SPI device, then
+ * xmit_data flag is set false.
+ */
+ a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+ }
+
+ while (a3700_spi->buf_len) {
+ if (a3700_spi->tx_buf) {
+ /* Wait wfifo ready */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_WFIFO_RDY)) {
+ dev_err(&spi->dev,
+ "wait wfifo ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+ /* Fill up the wfifo */
+ ret = a3700_spi_fifo_write(a3700_spi);
+ if (ret)
+ goto error;
+ } else if (a3700_spi->rx_buf) {
+ /* Wait rfifo ready */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_RFIFO_RDY)) {
+ dev_err(&spi->dev,
+ "wait rfifo ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+ /* Drain out the rfifo */
+ ret = a3700_spi_fifo_read(a3700_spi);
+ if (ret)
+ goto error;
+ }
+ }
+
+ /*
+ * Stop a write transfer in fifo mode:
+ * - wait all the bytes in wfifo to be shifted out
+ * - set XFER_STOP bit
+ * - wait XFER_START bit clear
+ * - clear XFER_STOP bit
+ * Stop a read transfer in fifo mode:
+ * - the hardware is to reset the XFER_START bit
+ * after the number of bytes indicated in DIN_CNT
+ * register
+ * - just wait XFER_START bit clear
+ */
+ if (a3700_spi->tx_buf) {
+ if (a3700_spi->xmit_data) {
+ /*
+ * If there are data written to the SPI device, wait
+ * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+ * transfer out of write FIFO.
+ */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_WFIFO_EMPTY)) {
+ dev_err(&spi->dev, "wait wfifo empty timed out\n");
+ return -ETIMEDOUT;
+ }
+ } else {
+ /*
+ * If the instruction in SPI_INSTR does not require data
+ * to be written to the SPI device, wait until SPI_RDY
+ * is 1 for the SPI interface to be in idle.
+ */
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait xfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ }
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_XFER_START))
+ break;
+ udelay(1);
+ }
+
+ if (timeout == 0) {
+ dev_err(&spi->dev, "wait transfer start clear timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+
+ val &= ~A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ goto out;
+
+error:
+ a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+ spi_finalize_current_transfer(master);
+
+ return ret;
+}
+
static int a3700_spi_unprepare_message(struct spi_master *master,
struct spi_message *message)
{
@@ -592,6 +964,23 @@ static int a3700_spi_probe(struct platform_device *pdev)
goto error;
}
+ if (of_device_is_compatible(of_node, "marvell,armada-3700-spi")) {
+ master->prepare_message = a3700_spi_prepare_fifo_message;
+ master->transfer_one = a3700_spi_fifo_transfer_one;
+
+ spi->flags |= HAS_FIFO;
+ spi->instr_cnt = A3700_INSTR_CNT;
+ spi->addr_cnt = A3700_ADDR_CNT;
+ spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+ A3700_DUMMY_CNT;
+ master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+ SPI_RX_QUAD | SPI_TX_QUAD);
+ } else {
+ master->prepare_message = a3700_spi_prepare_message;
+ master->transfer_one = a3700_spi_transfer_one;
+ master->unprepare_message = a3700_spi_unprepare_message;
+ }
+
ret = a3700_spi_init(spi);
if (ret)
goto error_clk;
--
2.9.3
--
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 v2 3/5] dt-bindings: spi: Add documentation for the Armada 3700 SPI Controller
From: Romain Perier @ 2016-11-30 9:43 UTC (permalink / raw)
To: Mark Brown, linux-spi
Cc: Mark Rutland, Andrew Lunn, Jason Cooper, Pawel Moll, devicetree,
Ian Campbell, Nadav Haklai, Rob Herring, Kumar Gala,
Gregory Clement, xigu, dingwei, Thomas Petazzoni,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>
This adds the devicetree bindings documentation for the SPI controller
present in the Marvell Armada 3700 SoCs.
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
.../devicetree/bindings/spi/spi-armada-3700.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-armada-3700.txt b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
new file mode 100644
index 0000000..1564aa8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
@@ -0,0 +1,25 @@
+* Marvell Armada 3700 SPI Controller
+
+Required Properties:
+
+- compatible: should be "marvell,armada-3700-spi"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- interrupts: The interrupt number. The interrupt specifier format depends on
+ the interrupt controller and of its driver.
+- clocks: Must contain the clock source, usually from the North Bridge clocks.
+- num-cs: The number of chip selects that is supported by this SPI Controller
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Example:
+
+ spi0: spi@10600 {
+ compatible = "marvell,armada-3700-spi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10600 0x5d>;
+ clocks = <&nb_perih_clk 7>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+ num-cs = <4>;
+ };
--
2.9.3
^ permalink raw reply related
* [PATCH v2 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700
From: Romain Perier @ 2016-11-30 9:43 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Armada 3700 SoC has an SPI Controller, this commit adds the definition
of the SPI device node at the SoC level.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v2:
- Removed properties max-frequency and clock-frequency, it is no
longer required and not used by the DT-bindings.
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..63c2002 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,17 @@
/* 32M internal register @ 0xd000_0000 */
ranges = <0x0 0x0 0xd0000000 0x2000000>;
+ spi0: spi@10600 {
+ compatible = "marvell,armada-3700-spi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10600 0x5d>;
+ clocks = <&nb_periph_clk 7>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+ num-cs = <4>;
+ status = "disabled";
+ };
+
uart0: serial@12000 {
compatible = "marvell,armada-3700-uart";
reg = <0x12000 0x400>;
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 v2 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
From: Romain Perier @ 2016-11-30 9:43 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161130094351.2748-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
This commit enables the device node spi0 on the official development
board for the Marvell Armada 3700. It also adds sub-node for the 128Mb
SPI-NOR present on the board.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 ++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..0c4eb98 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -67,6 +67,36 @@
status = "okay";
};
+&spi0 {
+ status = "okay";
+
+ m25p80@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <108000000>;
+ spi-rx-bus-width = <4>;
+ spi-tx-bus-width = <4>;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ partition@0 {
+ label = "bootloader";
+ reg = <0x0 0x200000>;
+ };
+ partition@200000 {
+ label = "U-boot Env";
+ reg = <0x200000 0x10000>;
+ };
+ partition@210000 {
+ label = "Linux";
+ reg = <0x210000 0xDF0000>;
+ };
+ };
+ };
+};
+
/* Exported on the micro USB connector CON32 through an FTDI */
&uart0 {
status = "okay";
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 1/6] net: ethernet: ti: netcp: add support of cpts
From: Richard Cochran @ 2016-11-30 9:44 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, Rob Herring, devicetree, Murali Karicheri,
Wingman Kwok
In-Reply-To: <20161128230428.6872-2-grygorii.strashko@ti.com>
On Mon, Nov 28, 2016 at 05:04:23PM -0600, Grygorii Strashko wrote:
> @@ -678,6 +744,9 @@ struct gbe_priv {
> int num_et_stats;
> /* Lock for updating the hwstats */
> spinlock_t hw_stats_lock;
> +
> + int cpts_registered;
The usage of this counter is racy.
> + struct cpts *cpts;
> };
This ++ and -- business ...
> +static void gbe_register_cpts(struct gbe_priv *gbe_dev)
> +{
> + if (!gbe_dev->cpts)
> + return;
> +
> + if (gbe_dev->cpts_registered > 0)
> + goto done;
> +
> + if (cpts_register(gbe_dev->cpts)) {
> + dev_err(gbe_dev->dev, "error registering cpts device\n");
> + return;
> + }
> +
> +done:
> + ++gbe_dev->cpts_registered;
> +}
> +
> +static void gbe_unregister_cpts(struct gbe_priv *gbe_dev)
> +{
> + if (!gbe_dev->cpts || (gbe_dev->cpts_registered <= 0))
> + return;
> +
> + if (--gbe_dev->cpts_registered)
> + return;
> +
> + cpts_unregister(gbe_dev->cpts);
> +}
is invoked from your open() and close() methods, but those methods
are not serialized among multiple ports.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v4 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-30 9:47 UTC (permalink / raw)
To: Florian Fainelli, netdev, devicetree
Cc: Andrew Lunn, Alexandre TORGUE, Neil Armstrong,
Martin Blumenstingl, Kevin Hilman, linux-kernel, Yegor Yefremov,
Julia Lawall, Andre Roth, linux-amlogic, Carlo Caione,
Giuseppe Cavallaro, Andreas Färber, linux-arm-kernel
In-Reply-To: <049b1efc-3bad-92e0-45ef-0563dc5d81de@gmail.com>
On Mon, 2016-11-28 at 09:54 -0800, Florian Fainelli wrote:
> On 11/28/2016 07:50 AM, Jerome Brunet wrote:
> >
> > This patchset fixes an issue with the OdroidC2 board (DWMAC +
> > RTL8211F).
> > The platform seems to enter LPI on the Rx path too often while
> > performing
> > relatively high TX transfer. This eventually break the link (both
> > Tx and
> > Rx), and require to bring the interface down and up again to get
> > the Rx
> > path working again.
> >
> > The root cause of this issue is not fully understood yet but
> > disabling EEE
> > advertisement on the PHY prevent this feature to be negotiated.
> > With this change, the link is stable and reliable, with the
> > expected
> > throughput performance.
> >
> > The patchset adds options in the generic phy driver to disable EEE
> > advertisement, through device tree. The way it is done is very
> > similar
> > to the handling of the max-speed property.
> >
> > Patch 4 is provided here for testing purpose only. Please don't
> > merge
> > patch 4, this change will go through the amlogic's tree.
>
> Sorry, but I really don't like the route this is going, and I should
> have made myself clearer before on that, I really think utilizing a
> PHY
> fixup is more appropriate here than an extremely generic DT property.
> The fixup code can be in the affected PHY driver, or it can be
> somewhere
> else, your call. There is no shortage of option on how to implement
> it,
> and this would be something easy to enable/disable for known good
> configurations (ala PCI/USB fixups).
>
> If we start supporting generic "enable", "disable" type of properties
> with values that map directly to register definitions of the HW, we
> leave too much room for these properties to be utilized to implement
> a
> specific policy, and this is not acceptable.
Florian,
I agree that DT should not be used to setup a policy, but to describe
what the HW is.
I tried to implement it the way you suggested, using phy fixup, too see
what it looks like.
There is 2 places in the code that seems (remotely) linked to the
issue:
- meson8b_dwmac driver : if the mac, regardless of the board/platform,
could not tolerate to have EEE activated, it would make sense to have
the fixup here. It can provide a C callback for such case.
- realtek phy driver: philosophy is kind of the same
To be clear, it is doable and it works that way, but I don't think
embedding this directly in the code is the right way to do it. It seems
we are hiding an information specific about the board inside a generic
driver.
We have several amlogic's design with the same MAC, sometimes with the
same PHY, which have no problem with EEE at all. The issue is really
about the board design.
What I propose is not an enable/disable configuration switch, but to
clearly state that a particular mode of operation is broken. Like the
"max-speed" property, it setup a restriction. IMO, this is a
description of what the HW is and is capable of, and as such it should
be part of the DT.
Yes the property directly map to a register, but it does let you
directly manipulate it (you can't pass the value you want to write in
the register). Having it this way just makes the code simple on both
ends (user and driver).
Yes people could start abusing this to setup policy. In the end, it is
our responsibility, as community, to make sure APIs are used in a
proper way, and not let it be used that way.
I'm open to suggestion on how improve the solution, maybe something
which could bring more confidence that property won't be misused.
Jerome
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] arm64: dts: zx: add zx296718's topcrm node
From: Baoyou Xie @ 2016-11-30 9:50 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, jun.nie-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
baoyou.xie-QSEj5FYQhm4dnm+yROfE0A,
xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
Enable topcrm clock node for zx296718, which is used for
CPU's frequency change.
Furthermore, this patch adds the CPU clock phandle in CPU's node
and uses operating-points-v2 to register operating points.
So it can be used by cpufreq-dt driver.
Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm64/boot/dts/zte/zx296718.dtsi | 43 +++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index 6b239a3..992158a 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -44,6 +44,7 @@
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/zx296718-clock.h>
/ {
compatible = "zte,zx296718";
@@ -81,6 +82,8 @@
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x0 0x0>;
enable-method = "psci";
+ clocks = <&topcrm A53_GATE>;
+ operating-points-v2 = <&cluster0_opp>;
};
cpu1: cpu@1 {
@@ -88,6 +91,7 @@
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x0 0x1>;
enable-method = "psci";
+ operating-points-v2 = <&cluster0_opp>;
};
cpu2: cpu@2 {
@@ -95,6 +99,7 @@
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x0 0x2>;
enable-method = "psci";
+ operating-points-v2 = <&cluster0_opp>;
};
cpu3: cpu@3 {
@@ -102,6 +107,38 @@
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x0 0x3>;
enable-method = "psci";
+ operating-points-v2 = <&cluster0_opp>;
+ };
+ };
+
+ cluster0_opp: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp@500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ opp-microvolt = <857000>;
+ clock-latency-ns = <500000>;
+ };
+ opp@648000000 {
+ opp-hz = /bits/ 64 <648000000>;
+ opp-microvolt = <857000>;
+ clock-latency-ns = <500000>;
+ };
+ opp@800000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <882000>;
+ clock-latency-ns = <500000>;
+ };
+ opp@1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt = <892000>;
+ clock-latency-ns = <500000>;
+ };
+ opp@1188000000 {
+ opp-hz = /bits/ 64 <1188000000>;
+ opp-microvolt = <1009000>;
+ clock-latency-ns = <500000>;
};
};
@@ -279,6 +316,12 @@
dma-requests = <32>;
};
+ topcrm: clock-controller@1461000 {
+ compatible = "zte,zx296718-topcrm";
+ reg = <0x01461000 0x1000>;
+ #clock-cells = <1>;
+ };
+
sysctrl: sysctrl@1463000 {
compatible = "zte,zx296718-sysctrl", "syscon";
reg = <0x1463000 0x1000>;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-11-30 9:52 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161130102757.9eec1f7f3377d0f4787e3829-GANU6spQydw@public.gmane.org>
Hi Jean-François,
On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
> On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
> >> Well, I don't see what this connector can be.
> >> May you give me a DT example?
> >
> > Sure.
> >
> > arch/arm/boot/dts/r8a7791-koelsch.dts
> >
> > /* HDMI encoder */
> >
> > hdmi@39 {
> > compatible = "adi,adv7511w";
> > reg = <0x39>;
> > interrupt-parent = <&gpio3>;
> > interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> >
> > adi,input-depth = <8>;
> > adi,input-colorspace = "rgb";
> > adi,input-clock = "1x";
> > adi,input-style = <1>;
> > adi,input-justification = "evenly";
> >
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > port@0 {
> > reg = <0>;
> > adv7511_in: endpoint {
> > remote-endpoint = <&du_out_rgb>;
> > };
> > };
> >
> > port@1 {
> > reg = <1>;
> > adv7511_out: endpoint {
> > remote-endpoint = <&hdmi_con>;
> > };
> > };
> > };
> >
> > };
> >
> > /* HDMI connector */
> >
> > hdmi-out {
> > compatible = "hdmi-connector";
> > type = "a";
> >
> > port {
> > hdmi_con: endpoint {
> > remote-endpoint = <&adv7511_out>;
> > };
> > };
> > };
>
> Hi Laurent,
>
> Sorry for I don't see the interest:
> - it is obvious that the HDMI connector is a 'hdmi-connector'!
It still has to be told to the drivers, they don't know how to identify a
connector by looking at it :-)
> - the physical connector type may be changed on any board by a soldering
> iron or a connector to connector cable.
Which is also true for any other component on the board. DT (and for that
matter any firmware description of the platform) isn't soldering-proof.
> - what does the software do with the connector type?
That's up to the software to decide, the DT bindings should describe the
hardware in the most accurate and usable way for the OS as possible. One of my
longer term goals is to add connector drivers to handle DDC and HPD when
they're not handled by the encoder (they are in the above example).
If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD
to a GPIO, we would have
hdmi-out {
compatible = "hdmi-connector";
type = "a";
/* I2C bus and GPIO references are made up for the example */
ddc-i2c-bus = <&i2c4>;
hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
port {
hdmi_con: endpoint {
remote-endpoint = <&adv7511_out>;
};
};
};
and both HPD and EDID reading should be handled by the connector driver.
> - why not to put the connector information in the HDMI device?
Because the connector is separate from the encoder. It's not uncommon
(depending on the encoder type) to have the encoder output connected to a non-
connector entity such as another chained encoder.
For example most LVDS encoders are connected to a panel, but I have a board
with the following encoders chain.
CRTC -- parallel RGB --> on-SoC LVDS encoder -- LVDS --> on-board LVDS decoder
-- parallel RGB --> HDMI encoder -- HDMI --> HDMI connector
I can't support that if the LVDS encoder driver hardcodes the assumption that
the encoder output is connected to a panel. This kind of usage might be less
common for HDMI but is certainly not inconceivable.
> And, if I follow you, the graph of ports could also be used to describe
> the way the various parts of the SoCs are powered, to describe the pin
> connections, to describe the USB connectors, to describe the board
> internal hubs and bridges...
It should be used where applicable, it's not meant as the only possible
hardware description for all pieces of the system.
--
Regards,
Laurent Pinchart
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Richard Cochran @ 2016-11-30 9:56 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230428.6872-3-grygorii.strashko-l0cyMroinI0@public.gmane.org>
On Mon, Nov 28, 2016 at 05:04:24PM -0600, Grygorii Strashko wrote:
> Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet
> Switch Subsystems, can control an external multiplexer that selects
> one of up to 32 clocks for time sync reference (RFTCLK). This feature
> can be configured through CPTS_RFTCLK_SEL register (offset: x08).
>
> Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present,
> will specify CPTS reference clock. The cpts_rftclk_sel should be
> omitted in DT if HW doesn't support this feature. The external fixed
> rate clocks can be defined in board files as "fixed-clock".
Can't you implement this using the clock tree, rather than an ad-hoc
DT property?
Thanks,
Richard
--
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 v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-30 10:03 UTC (permalink / raw)
To: zhangfei; +Cc: devicetree, Stephen Boyd, Arnd Bergmann, linux-arm-kernel
In-Reply-To: <bec21998-b30c-4076-62bf-6b24c9dd1dd6@linaro.org>
Am Freitag, den 25.11.2016, 20:08 +0800 schrieb zhangfei:
>
> On 2016年11月25日 18:54, Philipp Zabel wrote:
> > Am Freitag, den 25.11.2016, 18:42 +0800 schrieb zhangfei:
> >> On 2016年11月25日 18:25, Philipp Zabel wrote:
> >>> Am Donnerstag, den 24.11.2016, 18:20 +0800 schrieb zhangfei:
> >>>> On 2016年11月24日 17:50, Philipp Zabel wrote:
> >>>>> Am Donnerstag, den 24.11.2016, 17:40 +0800 schrieb zhangfei:
> >>>>>> On 2016年11月24日 17:26, Philipp Zabel wrote:
> >>>>>>> Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> >>>>>>>> Add DT bindings documentation for hi3660 SoC reset controller.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> >>>>>>>> ---
> >>>>>>>> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
> >>>>>>>> 1 file changed, 51 insertions(+)
> >>>>>>>> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>> new file mode 100644
> >>>>>>>> index 0000000..250daf2
> >>>>>>>> --- /dev/null
> >>>>>>>> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>> @@ -0,0 +1,51 @@
> >>>>>>>> +Hisilicon System Reset Controller
> >>>>>>>> +======================================
> >>>>>>>> +
> >>>>>>>> +Please also refer to reset.txt in this directory for common reset
> >>>>>>>> +controller binding usage.
> >>>>>>>> +
> >>>>>>>> +The reset controller registers are part of the system-ctl block on
> >>>>>>>> +hi3660 SoC.
> >>>>>>>> +
> >>>>>>>> +Required properties:
> >>>>>>>> +- compatible: should be
> >>>>>>>> + "hisilicon,hi3660-reset"
> >>>>>>>> +- #reset-cells: 1, see below
> >>>>>>>> +- hisi,rst-syscon: phandle of the reset's syscon.
> >>>>>>>> +- hisi,reset-bits: Contains the reset control register information
> >>>>>>>> + Should contain 2 cells for each reset exposed to
> >>>>>>>> + consumers, defined as:
> >>>>>>>> + Cell #1 : offset from the syscon register base
> >>>>>>>> + Cell #2 : bits position of the control register
> >>>>>>>> +
> >>>>>>>> +Example:
> >>>>>>>> + iomcu: iomcu@ffd7e000 {
> >>>>>>>> + compatible = "hisilicon,hi3660-iomcu", "syscon";
> >>>>>>>> + reg = <0x0 0xffd7e000 0x0 0x1000>;
> >>>>>>>> + };
> >>>>>>>> +
> >>>>>>>> + iomcu_rst: iomcu_rst_controller {
> >>>>>>> This should be
> >>>>>>> iomcu_rst: reset-controller {
> >>>>>>>
> >>>>>>>> + compatible = "hisilicon,hi3660-reset";
> >>>>>>>> + #reset-cells = <1>;
> >>>>>>>> + hisi,rst-syscon = <&iomcu>;
> >>>>>>>> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
> >>>>>>>> + 0x20 0x10 /* 1: i2c1 */
> >>>>>>>> + 0x20 0x20 /* 2: i2c2 */
> >>>>>>>> + 0x20 0x8000000>; /* 3: i2c6 */
> >>>>>>>> + };
> >>>>>>> The reset lines are controlled through iomcu bits, is there a reason not
> >>>>>>> to put the iomcu_rst node inside the iomcu node? That way the
> >>>>>>> hisi,rst-syscon property could be removed and the syscon could be
> >>>>>>> retrieved via the reset-controller parent node.
> >>>>>> iomcu is common registers, controls clock and reset, etc.
> >>>>>> So we use syscon, without mapping the registers everywhere.
> >>>>>> It is common case in hisilicon, same in hi6220.
> >>>>>>
> >>>>>> Also the #clock-cells and #reset-cells can not be put in the same node,
> >>>>>> if they are both using probe, since reset_probe will not be called.
> >>>>>>
> >>>>>> So we use hisi,rst-syscon as a general solution.
> >>>>> What I meant is this:
> >>>>>
> >>>>> iomcu: iomcu@ffd7e000 {
> >>>>> compatible = "hisilicon,hi3660-iomcu", "syscon", "simple-mfd";
> >>>>> reg = <0x0 0xffd7e000 0x0 0x1000>;
> >>>> #clock-cells = <1>;
> >>>>
> >>>> In my test, if there add #clock-cells = <1>, reset_probe will not be
> >>>> called any more.
> >>>> Since clk_probe is called first.
> >>>> No matter iomcu_rst is child node or not.
> >>> I don't understand this, does the clock driver bind to the iomcu node
> >>> using CLK_OF_DECLARE_DRIVER(..., "hisilicon,hi3660-iomcu", ...)?
> >> This method:CLK_OF_DECLARE_DRIVER is not prefered in clock,
> >> and we have to use probe instead, to make all driver build as modules as
> >> possible.
> >>
> >> For example hi3660.
> >> static struct platform_driver hi3660_clk_driver = {
> >> .probe = hi3660_clk_probe,
> >> .driver = {
> >> .name = "hi3660-clk",
> >> .of_match_table = hi3660_clk_match_table,
> >> },
> >> };
> > hi3660_clk_match_table contains the "hisilicon,hi3660-iomcu" compatible?
> > If so, you could call
> > of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> > from hi3660_clk_probe instead of using "simple-mfd" to probe the iomcu
> > node's children.
>
> Not using simple-mfd:
>
> Like
> static const struct of_device_id hi3660_clk_match_table[] = {
> { .compatible = "hisilicon,hi3660-iomcu", },
> { }
> };
> MODULE_DEVICE_TABLE(of, hi3660_clk_match_table);
>
> static int hi3660_clk_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = pdev->dev.of_node;
> const struct of_device_id *of_id;
> enum hi3660_clk_type type;
>
> of_id = of_match_device(hi3660_clk_match_table, dev);
> if (!of_id)
> return -EINVAL;
> ~
> }
>
> If put iomcu_rst as child node, and set #clock-cells = <1> to iomcu,
> then hi3660_clk_probe is called, hi3660_reset_probe will not be called.
For hi3660_reset_probe to be called, you'll have to call
of_platform_populate to probe the hi3660-iomcu children in this case.
> So using "hisi,rst-syscon" as pointer does not have the issue.
I understand that, it still sounds to me like you are organizing the
device tree around limitations of the current code. Instead the device
tree should be organized to best describe the hardware, and the code
should be adapted to support that.
Of course, if you use the flat DT layout everywhere else, I won't try to
block the reset driver because of this issue. I'm just saying nested
nodes in the DT would better describe the real control flow.
regards
Philipp
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] arm64: dts: juno: Correct PCI IO window
From: liviu.dudau @ 2016-11-30 10:04 UTC (permalink / raw)
To: Jeremy Linton
Cc: mark.rutland, devicetree, lorenzo.pieralisi, catalin.marinas,
will.deacon, robh+dt, sudeep.holla, linux-arm-kernel
In-Reply-To: <1480452310-29286-1-git-send-email-jeremy.linton@arm.com>
On Tue, Nov 29, 2016 at 02:45:10PM -0600, Jeremy Linton wrote:
> The PCIe root complex on Juno translates the MMIO mapped
> at 0x5f800000 to the PIO address range starting at 0
> (which is common because PIO addresses are generally < 64k).
> Correct the DT to reflect this.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
With the U-Boot patch that I have sent to the ML:
Tested-by: Liviu Dudau <Liviu.Dudau@arm.com>
also
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
Best regards,
Liviu
> ---
> arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 334271a..7d3a2ac 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -393,7 +393,7 @@
> #address-cells = <3>;
> #size-cells = <2>;
> dma-coherent;
> - ranges = <0x01000000 0x00 0x5f800000 0x00 0x5f800000 0x0 0x00800000>,
> + ranges = <0x01000000 0x00 0x00000000 0x00 0x5f800000 0x0 0x00800000>,
> <0x02000000 0x00 0x50000000 0x00 0x50000000 0x0 0x08000000>,
> <0x42000000 0x40 0x00000000 0x40 0x00000000 0x1 0x00000000>;
> #interrupt-cells = <1>;
> --
> 2.5.5
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Richard Cochran @ 2016-11-30 10:05 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, Rob Herring, devicetree, Murali Karicheri,
Wingman Kwok
In-Reply-To: <20161128230428.6872-5-grygorii.strashko@ti.com>
On Mon, Nov 28, 2016 at 05:04:26PM -0600, Grygorii Strashko wrote:
> The TS_COMP output in the CPSW CPTS module is asserted for
> ts_comp_length[15:0] RCLK periods when the time_stamp value compares
> with the ts_comp_val[31:0] and the length value is non-zero. The
> TS_COMP pulse edge occurs three RCLK periods after the values
> compare. A timestamp compare event is pushed into the event FIFO when
> TS_COMP is asserted.
>
> This patch adds support of Pulse-Per-Second (PPS) by using the
> timestamp compare output. The CPTS driver adds one second of counter
> value to the ts_comp_val register after each assertion of the TS_COMP
> output. The TS_COMP pulse polarity and width are configurable in DT.
I really dislike this patch. You go through contortions to get from
the timecounter back to the raw HW counter. That is rather ugly.
Can you adjust the frequency of the keystone devices in hardware? If
so, then please implement it, and just disable PPS for the CPSW.
The only reason I used the timecounter for frequency adjustment was
because the am335x HW is broken. But this shouldn't hold back other
newer HW without the same silicon flaws.
Thanks,
Richard
^ permalink raw reply
* Re: arm64: renesas: r8a7796/salvator-x: Add board part number to DT bindings
From: Simon Horman @ 2016-11-30 10:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Magnus Damm, linux-renesas-soc, linux-arm-kernel, devicetree
In-Reply-To: <1472635054-24372-1-git-send-email-geert+renesas@glider.be>
On Wed, Aug 31, 2016 at 11:17:34AM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Thanks, queued up for v4.11.
^ permalink raw reply
* Re: [PATCH] iio: misc: add a generic regulator driver
From: Lars-Peter Clausen @ 2016-11-30 10:10 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-devicetree, LKML, Kevin Hilman, Patrick Titiano,
Neil Armstrong
In-Reply-To: <CAMpxmJWDWk-mXUHHGEKm-VVwzqEGLDQjc274Xq+Qwc2eN=YbsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/29/2016 04:35 PM, Bartosz Golaszewski wrote:
> 2016-11-29 16:30 GMT+01:00 Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>:
>> On 11/29/2016 04:22 PM, Bartosz Golaszewski wrote:
>> [...]
>>> diff --git a/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt b/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt
>>> new file mode 100644
>>> index 0000000..147458f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt
>>> @@ -0,0 +1,18 @@
>>> +Industrial IO regulator device driver
>>> +-------------------------------------
>>> +
>>> +This document describes the bindings for the iio-regulator - a dummy device
>>> +driver representing a physical regulator within the iio framework.
>>
>> No bindings for drivers, only for hardware. So this wont work.
>>
>
> What about exporting regulator attributes analogous to the one in this
> patch from the iio-core when a *-supply property is specified for a
> node?
The problem with exposing direct control to the regulator is that it allows
to modify the hardware state without the drivers knowledge. If you
power-cycle a device all previous configuration that has been written to the
device is reset. The device driver needs to be aware of this otherwise its
assumed state and the actual device state can divert which will result in
undefined behavior. Also access to the device will fail unexpectedly when
the regulator is turned off. So I think generally the driver should
explicitly control the regulator, power-up when needed, power-down when not.
- Lars
--
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 0/8] drm: sun8i: Add DE2 HDMI video support
From: Laurent Pinchart @ 2016-11-30 10:12 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Jean-Francois Moine, Maxime Ripard,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161130100545.44546ed28896cdb9768f68cd-GANU6spQydw@public.gmane.org>
Hi Jean-François,
On Wednesday 30 Nov 2016 10:05:45 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 22:36:50 +0100 Maxime Ripard wrote:
> > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> > > This patchset series adds HDMI video support to the Allwinner
> > > sun8i SoCs which include the display engine 2 (DE2).
> > > The driver contains the code for the A83T and H3 SoCs, and
> > > some H3 boards, but it could be used/extended for other SoCs
> > > (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> >
> > Honestly, I'm getting a bit worried by the fact that you ignore
> > reviews.
> >
> > On the important reviews that you got that are to be seen as major
> >
> > issues that block the inclusion, we have:
> > - The fact that the HDMI driver is actually just a designware IP,
> > and while you should use the driver that already exists, you just
> > duplicated all that code.
>
> The DW registers in the A83T and H3 are obfuscated, so, the code in
> bridge/DW cannot be used as it is. There should be either a translation
> table or a function to compute the register addresses.
Jernej mentioned there could be a way to restore the original Synopsys memory
map. If that works then using the dw-hdmi could be an option.
> More, it is not sure that the bridge/DW code would work with Allwinner's
> SoCs.
If it doesn't work and can't be made to work in a non-invasive way they it
should certainly not be used :-)
> It seems that they got some schematics from DesignWare, but, is it really
> the same hardware?
That's not really relevant, as long as the hardware is compatible, it doesn't
matter if it's a Synopsys IP or a custom implementation of the HDMI TX with a
compatible interface.
> Also, if some changes had to be done in the bridge code, I could not check
> if this would break or not the other SoCs.
Nothing new here, all drivers that support multiple SoCs have the same
problem. That's why we have maintainers, testers and board farms to try and
catch issues as early as possible.
> Eventually, I went the same way as omap/hdmi5: different driver.
I might try to fix that for OMAP5 at some point, we'll see.
> > - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> > graph to model the connection between the display engine and the
> > TCON. Something that Laurent also pointed out in this version.
>
> I simply use the drm function drm_of_component_probe().
> If this one is in the DRM core, why should I not use it?
> If it must not be used, it would be nice to mark it as deprecated and
> to update the code of the drivers which are using it.
I haven't used that function so I can't comment here, except for the fact that
DT bindings are not designed based on a given OS implementation. It should be
the other way around, the bindings should be designed to clearly describe the
hardware in a clean and consistent way that can be used by any host software,
and the Linux helper functions should then be developed based on those
bindings.
> > - The fact that you ignored that you needed an HDMI connector node
> > as a child of the HDMI controller. This has been reported by Rob
> > (v6) and yet again in this version by Laurent.
>
> As I don't know what is a DT 'connector', I cannot go further.
> I hope Laurent will give me clearer explanations and a real example.
Done, we can discuss this in that part of the mail thread.
> > - And finally the fact that we can't have several display engine in
> > parallel, if needs be. This has happened in the past already on
> > Allwinner SoCs, so it's definitely something we should consider in
> > the DT bindings, since we can't break them.
>
> IIRC, I proposed my driver before yours, and the DE2 is completely
> different from the other display engines.
> What you are telling is "add more code to already complex code and have
> a big driver for all SoCs in each kernels".
> I think it should be better to have small modules, each one treating
> specific hardware, and to let only the needed code in the kernel memory
> at startup time.
>
> > Until those are fixed, I cannot see how this driver can be merged,
> > unfortunately.
>
> No problem. I just wanted to help people by giving the job I did on the
> boards I have. My boards are working for almost one year, fine enough
> for I use them as daily desktop computers. I don't want to spend one
> more year for having my code in the Linux kernel: there are so much
> other exciting things to do...
And you're certainly welcome to contribute drivers to the kernel, that's
always appreciated. Of course, to ensure a reasonable level of quality and
consistency between drivers, the review process often requires changes to be
made to the code being submitted. When it comes to drivers I mostly pay
attention to DT bindings, userspace APIs and modification to common code.
Driver code itself, as long as it's reasonably clean and doesn't impede
development of other drivers or impact system security in an adverse way, is
still important but maybe slightly less so. I'll defer to Maxime to come to an
agreement on the multiple display engines in parallel problem as I'm not
familiar with it for the Allwinner platforms.
--
Regards,
Laurent Pinchart
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 3/6] net: ethernet: ti: cpts: add support of cpts HW_TS_PUSH
From: Richard Cochran @ 2016-11-30 10:19 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230428.6872-4-grygorii.strashko-l0cyMroinI0@public.gmane.org>
On Mon, Nov 28, 2016 at 05:04:25PM -0600, Grygorii Strashko wrote:
> +/* HW TS */
> +static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
> +{
> + unsigned long flags;
> + u32 v;
> +
> + if (index >= cpts->info.n_ext_ts)
> + return -ENXIO;
> +
> + if (((cpts->hw_ts_enable & BIT(index)) >> index) == on)
> + return 0;
> +
> + spin_lock_irqsave(&cpts->lock, flags);
> +
> + v = cpts_read32(cpts, control);
> + if (on) {
> + v |= BIT(8 + index);
> + cpts->hw_ts_enable |= BIT(index);
> + } else {
> + v &= ~BIT(8 + index);
> + cpts->hw_ts_enable &= ~BIT(index);
> + }
> + cpts_write32(cpts, v, control);
> +
> + spin_unlock_irqrestore(&cpts->lock, flags);
> +
> + if (cpts->hw_ts_enable)
> + /* poll for events faster - evry 200 ms */
every
> + cpts->ov_check_period =
> + msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
Bad indentation. Use braces {} to contain the comment and assignment
statement.
> + else
> + cpts->ov_check_period = cpts->ov_check_period_slow;
> +
> + mod_delayed_work(system_wq, &cpts->overflow_work,
> + cpts->ov_check_period);
> +
> + return 0;
> +}
Thanks,
Richard
--
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox