* Re: [PATCH 3/8] rtc: add STM32 RTC driver
From: Mathieu Poirier @ 2016-12-05 16:32 UTC (permalink / raw)
To: Amelie DELAUNAY
Cc: a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Alexandre TORGUE, linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Gabriel FERNANDEZ
In-Reply-To: <15bea9e9-adcc-edb0-1bd1-33d395c72eec-qxv4g6HH51o@public.gmane.org>
On Mon, Dec 05, 2016 at 10:43:14AM +0100, Amelie DELAUNAY wrote:
> Hi Mathieu,
>
> Thanks for reviewing
>
> On 12/02/2016 06:56 PM, Mathieu Poirier wrote:
> > On Fri, Dec 02, 2016 at 03:09:56PM +0100, Amelie Delaunay wrote:
> >> This patch adds support for the STM32 RTC.
> >
> > Hello Amelie,
> >
> >>
> >> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> >> ---
> >> drivers/rtc/Kconfig | 10 +
> >> drivers/rtc/Makefile | 1 +
> >> drivers/rtc/rtc-stm32.c | 777
> ++++++++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 788 insertions(+)
> >> create mode 100644 drivers/rtc/rtc-stm32.c
> >>
> >> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> >> index e859d14..dd8b218 100644
> >> --- a/drivers/rtc/Kconfig
> >> +++ b/drivers/rtc/Kconfig
> >> @@ -1706,6 +1706,16 @@ config RTC_DRV_PIC32
> >> This driver can also be built as a module. If so, the module
> >> will be called rtc-pic32
> >>
> >> +config RTC_DRV_STM32
> >> + tristate "STM32 On-Chip RTC"
> >> + depends on ARCH_STM32
> >> + help
> >> + If you say yes here you get support for the STM32 On-Chip
> >> + Real Time Clock.
> >> +
> >> + This driver can also be built as a module, if so, the module
> >> + will be called "rtc-stm32".
> >> +
> >> comment "HID Sensor RTC drivers"
> >>
> >> config RTC_DRV_HID_SENSOR_TIME
> >> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> >> index 1ac694a..87bd9cc 100644
> >> --- a/drivers/rtc/Makefile
> >> +++ b/drivers/rtc/Makefile
> >> @@ -144,6 +144,7 @@ obj-$(CONFIG_RTC_DRV_SNVS) += rtc-snvs.o
> >> obj-$(CONFIG_RTC_DRV_SPEAR) += rtc-spear.o
> >> obj-$(CONFIG_RTC_DRV_STARFIRE) += rtc-starfire.o
> >> obj-$(CONFIG_RTC_DRV_STK17TA8) += rtc-stk17ta8.o
> >> +obj-$(CONFIG_RTC_DRV_STM32) += rtc-stm32.o
> >> obj-$(CONFIG_RTC_DRV_STMP) += rtc-stmp3xxx.o
> >> obj-$(CONFIG_RTC_DRV_ST_LPC) += rtc-st-lpc.o
> >> obj-$(CONFIG_RTC_DRV_SUN4V) += rtc-sun4v.o
> >> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
> >> new file mode 100644
> >> index 0000000..9e710ff
> >> --- /dev/null
> >> +++ b/drivers/rtc/rtc-stm32.c
> >> @@ -0,0 +1,777 @@
> >> +/*
> >> + * Copyright (C) Amelie Delaunay 2015
> >> + * Author: Amelie Delaunay <adelaunay.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> + * License terms: GNU General Public License (GPL), version 2
> >> + */
> >> +
> >> +#include <linux/bcd.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/init.h>
> >> +#include <linux/io.h>
> >> +#include <linux/iopoll.h>
> >> +#include <linux/ioport.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/rtc.h>
> >> +#include <linux/spinlock.h>
> >> +
> >> +#define DRIVER_NAME "stm32_rtc"
> >> +
> >> +/* STM32 RTC registers */
> >> +#define STM32_RTC_TR 0x00
> >> +#define STM32_RTC_DR 0x04
> >> +#define STM32_RTC_CR 0x08
> >> +#define STM32_RTC_ISR 0x0C
> >> +#define STM32_RTC_PRER 0x10
> >> +#define STM32_RTC_ALRMAR 0x1C
> >> +#define STM32_RTC_WPR 0x24
> >> +
> >> +/* STM32_RTC_TR bit fields */
> >> +#define STM32_RTC_TR_SEC_SHIFT 0
> >> +#define STM32_RTC_TR_SEC GENMASK(6, 0)
> >> +#define STM32_RTC_TR_MIN_SHIFT 8
> >> +#define STM32_RTC_TR_MIN GENMASK(14, 8)
> >> +#define STM32_RTC_TR_HOUR_SHIFT 16
> >> +#define STM32_RTC_TR_HOUR GENMASK(21, 16)
> >> +
> >> +/* STM32_RTC_DR bit fields */
> >> +#define STM32_RTC_DR_DATE_SHIFT 0
> >> +#define STM32_RTC_DR_DATE GENMASK(5, 0)
> >> +#define STM32_RTC_DR_MONTH_SHIFT 8
> >> +#define STM32_RTC_DR_MONTH GENMASK(11, 8)
> >> +#define STM32_RTC_DR_WDAY_SHIFT 13
> >> +#define STM32_RTC_DR_WDAY GENMASK(15, 13)
> >> +#define STM32_RTC_DR_YEAR_SHIFT 16
> >> +#define STM32_RTC_DR_YEAR GENMASK(23, 16)
> >> +
> >> +/* STM32_RTC_CR bit fields */
> >> +#define STM32_RTC_CR_FMT BIT(6)
> >> +#define STM32_RTC_CR_ALRAE BIT(8)
> >> +#define STM32_RTC_CR_ALRAIE BIT(12)
> >> +
> >> +/* STM32_RTC_ISR bit fields */
> >> +#define STM32_RTC_ISR_ALRAWF BIT(0)
> >> +#define STM32_RTC_ISR_INITS BIT(4)
> >> +#define STM32_RTC_ISR_RSF BIT(5)
> >> +#define STM32_RTC_ISR_INITF BIT(6)
> >> +#define STM32_RTC_ISR_INIT BIT(7)
> >> +#define STM32_RTC_ISR_ALRAF BIT(8)
> >> +
> >> +/* STM32_RTC_PRER bit fields */
> >> +#define STM32_RTC_PRER_PRED_S_SHIFT 0
> >> +#define STM32_RTC_PRER_PRED_S GENMASK(14, 0)
> >> +#define STM32_RTC_PRER_PRED_A_SHIFT 16
> >> +#define STM32_RTC_PRER_PRED_A GENMASK(22, 16)
> >> +
> >> +/* STM32_RTC_ALRMAR and STM32_RTC_ALRMBR bit fields */
> >> +#define STM32_RTC_ALRMXR_SEC_SHIFT 0
> >> +#define STM32_RTC_ALRMXR_SEC GENMASK(6, 0)
> >> +#define STM32_RTC_ALRMXR_SEC_MASK BIT(7)
> >> +#define STM32_RTC_ALRMXR_MIN_SHIFT 8
> >> +#define STM32_RTC_ALRMXR_MIN GENMASK(14, 8)
> >> +#define STM32_RTC_ALRMXR_MIN_MASK BIT(15)
> >> +#define STM32_RTC_ALRMXR_HOUR_SHIFT 16
> >> +#define STM32_RTC_ALRMXR_HOUR GENMASK(21, 16)
> >> +#define STM32_RTC_ALRMXR_PM BIT(22)
> >> +#define STM32_RTC_ALRMXR_HOUR_MASK BIT(23)
> >> +#define STM32_RTC_ALRMXR_DATE_SHIFT 24
> >> +#define STM32_RTC_ALRMXR_DATE GENMASK(29, 24)
> >> +#define STM32_RTC_ALRMXR_WDSEL BIT(30)
> >> +#define STM32_RTC_ALRMXR_WDAY_SHIFT 24
> >> +#define STM32_RTC_ALRMXR_WDAY GENMASK(27, 24)
> >> +#define STM32_RTC_ALRMXR_DATE_MASK BIT(31)
> >> +
> >> +/* STM32_RTC_WPR key constants */
> >> +#define RTC_WPR_1ST_KEY 0xCA
> >> +#define RTC_WPR_2ND_KEY 0x53
> >> +#define RTC_WPR_WRONG_KEY 0xFF
> >> +
> >> +/*
> >> + * RTC registers are protected agains parasitic write access.
> >> + * PWR_CR_DBP bit must be set to enable write access to RTC registers.
> >> + */
> >> +/* STM32_PWR_CR */
> >> +#define PWR_CR 0x00
> >> +/* STM32_PWR_CR bit field */
> >> +#define PWR_CR_DBP BIT(8)
> >> +
> >> +static struct regmap *dbp;
> >> +
> >> +struct stm32_rtc {
> >> + struct rtc_device *rtc_dev;
> >> + void __iomem *base;
> >> + struct clk *pclk;
> >> + struct clk *ck_rtc;
> >> + unsigned int clksrc;
> >> + spinlock_t lock; /* Protects registers accesses */
> >> + int irq_alarm;
> >> + struct regmap *pwrcr;
> >> +};
> >> +
> >> +static inline unsigned int stm32_rtc_readl(struct stm32_rtc *rtc,
> >> + unsigned int offset)
> >> +{
> >> + return readl_relaxed(rtc->base + offset);
> >> +}
> >> +
> >> +static inline void stm32_rtc_writel(struct stm32_rtc *rtc,
> >> + unsigned int offset, unsigned int value)
> >> +{
> >> + writel_relaxed(value, rtc->base + offset);
> >> +}
> >
> > I'm not sure wrapping the readl/writel_relaxed function does anything
> special
> > other than simply redirecting the reader to another section of the code.
> During development phase, it is useful to add debug traces but you're right,
> this can be remove.
> >
> >> +
> >> +static void stm32_rtc_wpr_unlock(struct stm32_rtc *rtc)
> >> +{
> >> +// if (dbp)
> >> +// regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
> >
> > Did checkpatch let you get away with this? What did you intend to do
> here?
> Hum, as surprising as it may seem, checkpatch didn't complained about these
> comments! But anyway, this has to be removed, it was a tentative to
> enable/disable backup domain write protection any time we have to write in a
> protected RTC register, but it is not functionnal. I have commented this
> just to keep it in mind and forget to remove it before sending.
> >
> >> +
> >> + stm32_rtc_writel(rtc, STM32_RTC_WPR, RTC_WPR_1ST_KEY);
> >> + stm32_rtc_writel(rtc, STM32_RTC_WPR, RTC_WPR_2ND_KEY);
> >> +}
> >> +
> >> +static void stm32_rtc_wpr_lock(struct stm32_rtc *rtc)
> >> +{
> >> + stm32_rtc_writel(rtc, STM32_RTC_WPR, RTC_WPR_WRONG_KEY);
> >> +
> >> +// if (dbp)
> >> +// regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP);
> >> +}
> >> +
> >> +static int stm32_rtc_enter_init_mode(struct stm32_rtc *rtc)
> >> +{
> >> + unsigned int isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
> >> +
> >> + if (!(isr & STM32_RTC_ISR_INITF)) {
> >> + isr |= STM32_RTC_ISR_INIT;
> >> + stm32_rtc_writel(rtc, STM32_RTC_ISR, isr);
> >> +
> >> + return readl_relaxed_poll_timeout_atomic(
> >> + rtc->base + STM32_RTC_ISR,
> >> + isr, (isr & STM32_RTC_ISR_INITF),
> >> + 10, 100000);
> >
> > When using hard coded numerics please add comments that explains the
> reason
> > behind the selected values.
> Sure. It takes around 2 RTCCLK clock cycles to enter in initialization phase
> mode. So it depends on the frequency of the ck_rtc parent clock.
> Either I keep parent clock frequency and compute the exact timeout, or I use
> the "best and worst cases": slowest RTCCLK frequency is 32kHz, so it can
> take up to 62us, highest RTCCLK frequency should be 1MHz, so it can take
> only 2us. Polling every 10us with a timeout of 100ms seemed reasonable and
> be a good compromise.
I think this is a resonnable approach - please add that explanation as a comment
in the code.
> >
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void stm32_rtc_exit_init_mode(struct stm32_rtc *rtc)
> >> +{
> >> + unsigned int isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
> >> +
> >> + isr &= ~STM32_RTC_ISR_INIT;
> >> + stm32_rtc_writel(rtc, STM32_RTC_ISR, isr);
> >> +}
> >> +
> >> +static int stm32_rtc_wait_sync(struct stm32_rtc *rtc)
> >> +{
> >> + unsigned int isr;
> >> +
> >> + isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
> >> +
> >> + isr &= ~STM32_RTC_ISR_RSF;
> >> + stm32_rtc_writel(rtc, STM32_RTC_ISR, isr);
> >> +
> >> + /* Wait the registers to be synchronised */
> >> + return readl_relaxed_poll_timeout_atomic(rtc->base + STM32_RTC_ISR,
> >> + isr,
> >> + (isr & STM32_RTC_ISR_RSF),
> >> + 10, 100000);
> >
> > Shouldn't the break condition be !((isr & STM32_RTC_ISR_RSF) ? If not
> this
> > probably deserve a better comment.
> RSF bit is set by hardware each time the calendar registers are synchronized
> (it takes up to 2 RTCCLK). So the break condition is correct: we poll until
> RSF flag is set or timeout is reached.
> >
> >> +}
> >> +
> >> +static irqreturn_t stm32_rtc_alarm_irq(int irq, void *dev_id)
> >> +{
> >> + struct stm32_rtc *rtc = (struct stm32_rtc *)dev_id;
> >> + unsigned long irqflags, events = 0;
> >> + unsigned int isr, cr;
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
> >> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
> >> +
> >> + if ((isr & STM32_RTC_ISR_ALRAF) &&
> >> + (cr & STM32_RTC_CR_ALRAIE)) {
> >> + /* Alarm A flag - Alarm interrupt */
> >> + events |= RTC_IRQF | RTC_AF;
> >> + isr &= ~STM32_RTC_ISR_ALRAF;
> >> + }
> >> +
> >> + /* Clear event irqflags, otherwise new events won't be received */
> >> + stm32_rtc_writel(rtc, STM32_RTC_ISR, isr);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + if (events) {
> >> + dev_info(&rtc->rtc_dev->dev, "Alarm occurred\n");
> >> +
> >> + /* Pass event to the kernel */
> >> + rtc_update_irq(rtc->rtc_dev, 1, events);
> >> + return IRQ_HANDLED;
> >> + } else {
> >> + return IRQ_NONE;
> >> + }
> >> +}
> >> +
> >> +/* Convert rtc_time structure from bin to bcd format */
> >> +static void tm2bcd(struct rtc_time *tm)
> >> +{
> >> + tm->tm_sec = bin2bcd(tm->tm_sec);
> >> + tm->tm_min = bin2bcd(tm->tm_min);
> >> + tm->tm_hour = bin2bcd(tm->tm_hour);
> >> +
> >> + tm->tm_mday = bin2bcd(tm->tm_mday);
> >> + tm->tm_mon = bin2bcd(tm->tm_mon + 1);
> >> + tm->tm_year = bin2bcd(tm->tm_year - 100);
> >> + /*
> >> + * Number of days since Sunday
> >> + * - on kernel side, 0=Sunday...6=Saturday
> >> + * - on rtc side, 0=invalid,1=Monday...7=Sunday
> >> + */
> >> + tm->tm_wday = (!tm->tm_wday) ? 7 : tm->tm_wday;
> >> +}
> >> +
> >> +/* Convert rtc_time structure from bcd to bin format */
> >> +static void bcd2tm(struct rtc_time *tm)
> >> +{
> >> + tm->tm_sec = bcd2bin(tm->tm_sec);
> >> + tm->tm_min = bcd2bin(tm->tm_min);
> >> + tm->tm_hour = bcd2bin(tm->tm_hour);
> >> +
> >> + tm->tm_mday = bcd2bin(tm->tm_mday);
> >> + tm->tm_mon = bcd2bin(tm->tm_mon) - 1;
> >> + tm->tm_year = bcd2bin(tm->tm_year) + 100;
> >> + /*
> >> + * Number of days since Sunday
> >> + * - on kernel side, 0=Sunday...6=Saturday
> >> + * - on rtc side, 0=invalid,1=Monday...7=Sunday
> >> + */
> >> + tm->tm_wday %= 7;
> >> +}
> >> +
> >> +static int stm32_rtc_read_time(struct device *dev, struct rtc_time *tm)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> + unsigned int tr, dr;
> >> + unsigned long irqflags;
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + /* Time and Date in BCD format */
> >> + tr = stm32_rtc_readl(rtc, STM32_RTC_TR);
> >> + dr = stm32_rtc_readl(rtc, STM32_RTC_DR);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + tm->tm_sec = (tr & STM32_RTC_TR_SEC) >> STM32_RTC_TR_SEC_SHIFT;
> >> + tm->tm_min = (tr & STM32_RTC_TR_MIN) >> STM32_RTC_TR_MIN_SHIFT;
> >> + tm->tm_hour = (tr & STM32_RTC_TR_HOUR) >> STM32_RTC_TR_HOUR_SHIFT;
> >> +
> >> + tm->tm_mday = (dr & STM32_RTC_DR_DATE) >> STM32_RTC_DR_DATE_SHIFT;
> >> + tm->tm_mon = (dr & STM32_RTC_DR_MONTH) >> STM32_RTC_DR_MONTH_SHIFT;
> >> + tm->tm_year = (dr & STM32_RTC_DR_YEAR) >> STM32_RTC_DR_YEAR_SHIFT;
> >> + tm->tm_wday = (dr & STM32_RTC_DR_WDAY) >> STM32_RTC_DR_WDAY_SHIFT;
> >> +
> >> + /* We don't report tm_yday and tm_isdst */
> >> +
> >> + bcd2tm(tm);
> >> +
> >> + if (rtc_valid_tm(tm) < 0) {
> >> + dev_err(dev, "%s: rtc_time is not valid.\n", __func__);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stm32_rtc_set_time(struct device *dev, struct rtc_time *tm)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> + unsigned int tr, dr;
> >> + unsigned long irqflags;
> >> + int ret = 0;
> >> +
> >> + if (rtc_valid_tm(tm) < 0) {
> >> + dev_err(dev, "%s: rtc_time is not valid.\n", __func__);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + tm2bcd(tm);
> >> +
> >> + /* Time in BCD format */
> >> + tr = ((tm->tm_sec << STM32_RTC_TR_SEC_SHIFT) & STM32_RTC_TR_SEC) |
> >> + ((tm->tm_min << STM32_RTC_TR_MIN_SHIFT) & STM32_RTC_TR_MIN) |
> >> + ((tm->tm_hour << STM32_RTC_TR_HOUR_SHIFT) & STM32_RTC_TR_HOUR);
> >> +
> >> + /* Date in BCD format */
> >> + dr = ((tm->tm_mday << STM32_RTC_DR_DATE_SHIFT) & STM32_RTC_DR_DATE)
> |
> >> + ((tm->tm_mon << STM32_RTC_DR_MONTH_SHIFT) & STM32_RTC_DR_MONTH)
> |
> >> + ((tm->tm_year << STM32_RTC_DR_YEAR_SHIFT) & STM32_RTC_DR_YEAR)
> |
> >> + ((tm->tm_wday << STM32_RTC_DR_WDAY_SHIFT) & STM32_RTC_DR_WDAY);
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + stm32_rtc_wpr_unlock(rtc);
> >> +
> >> + ret = stm32_rtc_enter_init_mode(rtc);
> >> + if (ret) {
> >> + dev_err(dev, "Can't enter in init mode. Set time aborted.\n");
> >> + goto end;
> >> + }
> >> +
> >> + stm32_rtc_writel(rtc, STM32_RTC_TR, tr);
> >> + stm32_rtc_writel(rtc, STM32_RTC_DR, dr);
> >> +
> >> + stm32_rtc_exit_init_mode(rtc);
> >> +
> >> + ret = stm32_rtc_wait_sync(rtc);
> >> +end:
> >> + stm32_rtc_wpr_lock(rtc);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int stm32_rtc_read_alarm(struct device *dev, struct rtc_wkalrm
> *alrm)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> + struct rtc_time *tm = &alrm->time;
> >> + unsigned int alrmar, cr, isr;
> >> + unsigned long irqflags;
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + alrmar = stm32_rtc_readl(rtc, STM32_RTC_ALRMAR);
> >> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
> >> + isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + if (alrmar & STM32_RTC_ALRMXR_DATE_MASK) {
> >> + /*
> >> + * Date/day don't care in Alarm comparison so alarm triggers
> >> + * every day
> >> + */
> >> + tm->tm_mday = -1;
> >> + tm->tm_wday = -1;
> >> + } else {
> >> + if (alrmar & STM32_RTC_ALRMXR_WDSEL) {
> >> + /* Alarm is set to a day of week */
> >> + tm->tm_mday = -1;
> >> + tm->tm_wday = (alrmar & STM32_RTC_ALRMXR_WDAY) >>
> >> + STM32_RTC_ALRMXR_WDAY_SHIFT;
> >> + tm->tm_wday %= 7;
> >> + } else {
> >> + /* Alarm is set to a day of month */
> >> + tm->tm_wday = -1;
> >> + tm->tm_mday = (alrmar & STM32_RTC_ALRMXR_DATE) >>
> >> + STM32_RTC_ALRMXR_DATE_SHIFT;
> >> + }
> >> + }
> >> +
> >> + if (alrmar & STM32_RTC_ALRMXR_HOUR_MASK) {
> >> + /* Hours don't care in Alarm comparison */
> >> + tm->tm_hour = -1;
> >> + } else {
> >> + tm->tm_hour = (alrmar & STM32_RTC_ALRMXR_HOUR) >>
> >> + STM32_RTC_ALRMXR_HOUR_SHIFT;
> >> + if (alrmar & STM32_RTC_ALRMXR_PM)
> >> + tm->tm_hour += 12;
> >> + }
> >> +
> >> + if (alrmar & STM32_RTC_ALRMXR_MIN_MASK) {
> >> + /* Minutes don't care in Alarm comparison */
> >> + tm->tm_min = -1;
> >> + } else {
> >> + tm->tm_min = (alrmar & STM32_RTC_ALRMXR_MIN) >>
> >> + STM32_RTC_ALRMXR_MIN_SHIFT;
> >> + }
> >> +
> >> + if (alrmar & STM32_RTC_ALRMXR_SEC_MASK) {
> >> + /* Seconds don't care in Alarm comparison */
> >> + tm->tm_sec = -1;
> >> + } else {
> >> + tm->tm_sec = (alrmar & STM32_RTC_ALRMXR_SEC) >>
> >> + STM32_RTC_ALRMXR_SEC_SHIFT;
> >> + }
> >> +
> >> + bcd2tm(tm);
> >> +
> >> + alrm->enabled = (cr & STM32_RTC_CR_ALRAE) ? 1 : 0;
> >> + alrm->pending = (isr & STM32_RTC_ISR_ALRAF) ? 1 : 0;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stm32_rtc_alarm_irq_enable(struct device *dev, unsigned int
> enabled)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> + unsigned long irqflags;
> >> + unsigned int isr, cr;
> >> +
> >> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
> >
> > Is the STM32_RTC_CR garanteed to be valid, i.e updated atomically? If not
> this
> > should probably be below the spinlock.
> You're right.
> >
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + stm32_rtc_wpr_unlock(rtc);
> >> +
> >> + /* We expose Alarm A to the kernel */
> >> + if (enabled)
> >> + cr |= (STM32_RTC_CR_ALRAIE | STM32_RTC_CR_ALRAE);
> >> + else
> >> + cr &= ~(STM32_RTC_CR_ALRAIE | STM32_RTC_CR_ALRAE);
> >> + stm32_rtc_writel(rtc, STM32_RTC_CR, cr);
> >> +
> >> + /* Clear event irqflags, otherwise new events won't be received */
> >> + isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
> >> + isr &= ~STM32_RTC_ISR_ALRAF;
> >> + stm32_rtc_writel(rtc, STM32_RTC_ISR, isr);
> >> +
> >> + stm32_rtc_wpr_lock(rtc);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stm32_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
> *alrm)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> + struct rtc_time *tm = &alrm->time;
> >> + unsigned long irqflags;
> >> + unsigned int cr, isr, alrmar;
> >> + int ret = 0;
> >> +
> >> + if (rtc_valid_tm(tm)) {
> >> + dev_err(dev, "Alarm time not valid.\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + tm2bcd(tm);
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + stm32_rtc_wpr_unlock(rtc);
> >> +
> >> + /* Disable Alarm */
> >> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
> >> + cr &= ~STM32_RTC_CR_ALRAE;
> >> + stm32_rtc_writel(rtc, STM32_RTC_CR, cr);
> >> +
> >> + /* Poll Alarm write flag to be sure that Alarm update is allowed */
> >> + ret = readl_relaxed_poll_timeout_atomic(rtc->base + STM32_RTC_ISR,
> >> + isr,
> >> + (isr & STM32_RTC_ISR_ALRAWF),
> >> + 10, 100);
> >> +
> >> + if (ret) {
> >> + dev_err(dev, "Alarm update not allowed\n");
> >> + goto end;
> >> + }
> >> +
> >> + alrmar = 0;
> >> +
> >> + if (tm->tm_mday < 0 && tm->tm_wday < 0) {
> >> + /*
> >> + * Date/day don't care in Alarm comparison so alarm triggers
> >> + * every day
> >> + */
> >> + alrmar |= STM32_RTC_ALRMXR_DATE_MASK;
> >> + } else {
> >> + if (tm->tm_mday > 0) {
> >> + /* Date is selected (ignoring wday) */
> >> + alrmar |= (tm->tm_mday << STM32_RTC_ALRMXR_DATE_SHIFT) &
> >> + STM32_RTC_ALRMXR_DATE;
> >> + } else {
> >> + /* Day of week is selected */
> >> + int wday = (tm->tm_wday == 0) ? 7 : tm->tm_wday;
> >> +
> >> + alrmar |= STM32_RTC_ALRMXR_WDSEL;
> >> + alrmar |= (wday << STM32_RTC_ALRMXR_WDAY_SHIFT) &
> >> + STM32_RTC_ALRMXR_WDAY;
> >> + }
> >> + }
> >> +
> >> + if (tm->tm_hour < 0) {
> >> + /* Hours don't care in Alarm comparison */
> >> + alrmar |= STM32_RTC_ALRMXR_HOUR_MASK;
> >> + } else {
> >> + /* 24-hour format */
> >> + alrmar &= ~STM32_RTC_ALRMXR_PM;
> >> + alrmar |= (tm->tm_hour << STM32_RTC_ALRMXR_HOUR_SHIFT) &
> >> + STM32_RTC_ALRMXR_HOUR;
> >> + }
> >> +
> >> + if (tm->tm_min < 0) {
> >> + /* Minutes don't care in Alarm comparison */
> >> + alrmar |= STM32_RTC_ALRMXR_MIN_MASK;
> >> + } else {
> >> + alrmar |= (tm->tm_min << STM32_RTC_ALRMXR_MIN_SHIFT) &
> >> + STM32_RTC_ALRMXR_MIN;
> >> + }
> >> +
> >> + if (tm->tm_sec < 0) {
> >> + /* Seconds don't care in Alarm comparison */
> >> + alrmar |= STM32_RTC_ALRMXR_SEC_MASK;
> >> + } else {
> >> + alrmar |= (tm->tm_sec << STM32_RTC_ALRMXR_SEC_SHIFT) &
> >> + STM32_RTC_ALRMXR_SEC;
> >> + }
> >> +
> >> + /* Write to Alarm register */
> >> + stm32_rtc_writel(rtc, STM32_RTC_ALRMAR, alrmar);
> >> +
> >> + if (alrm->enabled)
> >> + stm32_rtc_alarm_irq_enable(dev, 1);
> >> + else
> >> + stm32_rtc_alarm_irq_enable(dev, 0);
> >> +
> >> +end:
> >> + stm32_rtc_wpr_lock(rtc);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static const struct rtc_class_ops stm32_rtc_ops = {
> >> + .read_time = stm32_rtc_read_time,
> >> + .set_time = stm32_rtc_set_time,
> >> + .read_alarm = stm32_rtc_read_alarm,
> >> + .set_alarm = stm32_rtc_set_alarm,
> >> + .alarm_irq_enable = stm32_rtc_alarm_irq_enable,
> >> +};
> >> +
> >> +#ifdef CONFIG_OF
> >> +static const struct of_device_id stm32_rtc_of_match[] = {
> >> + { .compatible = "st,stm32-rtc" },
> >> + {}
> >> +};
> >> +MODULE_DEVICE_TABLE(of, stm32_rtc_of_match);
> >> +#endif
> >> +
> >> +static int stm32_rtc_init(struct platform_device *pdev,
> >> + struct stm32_rtc *rtc)
> >> +{
> >> + unsigned int prer, pred_a, pred_s, pred_a_max, pred_s_max, cr;
> >> + unsigned int rate;
> >> + unsigned long irqflags;
> >> + int ret = 0;
> >> +
> >> + rate = clk_get_rate(rtc->ck_rtc);
> >> +
> >> + /* Find prediv_a and prediv_s to obtain the 1Hz calendar clock */
> >> + pred_a_max = STM32_RTC_PRER_PRED_A >> STM32_RTC_PRER_PRED_A_SHIFT;
> >> + pred_s_max = STM32_RTC_PRER_PRED_S >> STM32_RTC_PRER_PRED_S_SHIFT;
> >> +
> >> + for (pred_a = pred_a_max; pred_a >= 0; pred_a--) {
> >> + pred_s = (rate / (pred_a + 1)) - 1;
> >> +
> >> + if (((pred_s + 1) * (pred_a + 1)) == rate)
> >> + break;
> >> + }
> >> +
> >> + /*
> >> + * Can't find a 1Hz, so give priority to RTC power consumption
> >> + * by choosing the higher possible value for prediv_a
> >> + */
> >> + if ((pred_s > pred_s_max) || (pred_a > pred_a_max)) {
> >> + pred_a = pred_a_max;
> >> + pred_s = (rate / (pred_a + 1)) - 1;
> >> +
> >> + dev_warn(&pdev->dev, "ck_rtc is %s\n",
> >> + (rate - ((pred_a + 1) * (pred_s + 1)) < 0) ?
> >> + "fast" : "slow");
> >> + }
> >> +
> >> + spin_lock_irqsave(&rtc->lock, irqflags);
> >> +
> >> + stm32_rtc_wpr_unlock(rtc);
> >> +
> >> + ret = stm32_rtc_enter_init_mode(rtc);
> >> + if (ret) {
> >> + dev_err(&pdev->dev,
> >> + "Can't enter in init mode. Prescaler config failed.\n");
> >> + goto end;
> >> + }
> >> +
> >> + prer = (pred_s << STM32_RTC_PRER_PRED_S_SHIFT) &
> STM32_RTC_PRER_PRED_S;
> >> + stm32_rtc_writel(rtc, STM32_RTC_PRER, prer);
> >> + prer |= (pred_a << STM32_RTC_PRER_PRED_A_SHIFT) &
> STM32_RTC_PRER_PRED_A;
> >> + stm32_rtc_writel(rtc, STM32_RTC_PRER, prer);
> >> +
> >> + /* Force 24h time format */
> >> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
> >> + cr &= ~STM32_RTC_CR_FMT;
> >> + stm32_rtc_writel(rtc, STM32_RTC_CR, cr);
> >> +
> >> + stm32_rtc_exit_init_mode(rtc);
> >> +
> >> + ret = stm32_rtc_wait_sync(rtc);
> >> +
> >> + if (stm32_rtc_readl(rtc, STM32_RTC_ISR) & STM32_RTC_ISR_INITS)
> >> + dev_warn(&pdev->dev, "Date/Time must be initialized\n");
> >> +end:
> >> + stm32_rtc_wpr_lock(rtc);
> >> +
> >> + spin_unlock_irqrestore(&rtc->lock, irqflags);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int stm32_rtc_probe(struct platform_device *pdev)
> >> +{
> >> + struct stm32_rtc *rtc;
> >> + struct resource *res;
> >> + int ret;
> >> +
> >> + rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> >> + if (!rtc)
> >> + return -ENOMEM;
> >> +
> >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >
> > The value of 'res' should be checked before using it.
> res is checked in devm_ioremap_resource just below :
> if (!res || resource_type(res) != IORESOURCE_MEM) {
> dev_err(dev, "invalid resource\n");
> return IOMEM_ERR_PTR(-EINVAL);
> }
> That's why it is not checked here.
> >
> >> + rtc->base = devm_ioremap_resource(&pdev->dev, res);
> >> + if (IS_ERR(rtc->base))
> >> + return PTR_ERR(rtc->base);
> >> +
> >> + dbp = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> "st,syscfg");
> >> + if (IS_ERR(dbp)) {
> >> + dev_err(&pdev->dev, "no st,syscfg\n");
> >> + return PTR_ERR(dbp);
> >> + }
> >> +
> >> + spin_lock_init(&rtc->lock);
> >> +
> >> + rtc->ck_rtc = devm_clk_get(&pdev->dev, "ck_rtc");
> >> + if (IS_ERR(rtc->ck_rtc)) {
> >> + dev_err(&pdev->dev, "no ck_rtc clock");
> >> + return PTR_ERR(rtc->ck_rtc);
> >> + }
> >> +
> >> + ret = clk_prepare_enable(rtc->ck_rtc);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (dbp)
> >> + regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
> >
> > The code above exits if there is a problem with the dbp, there is no point
> in
> > checking again.
> You're right.
> >
> >> +
> >> + ret = stm32_rtc_init(pdev, rtc);
> >> + if (ret)
> >> + goto err;
> >> +
> >> + rtc->irq_alarm = platform_get_irq_byname(pdev, "alarm");
> >> + if (rtc->irq_alarm <= 0) {
> >> + dev_err(&pdev->dev, "no alarm irq\n");
> >> + ret = -ENOENT;
> >> + goto err;
> >> + }
> >> +
> >> + platform_set_drvdata(pdev, rtc);
> >> +
> >> + device_init_wakeup(&pdev->dev, true);
> >
> > What happens if device_init_wakeup() returns an error?
> It means that RTC won't be able to wake up the board with RTC alarm. I can
> add a warning for the user in this case ?
Not really sure - it really depends on the kind of system will use this.
For some not being able to wake up the board might a minor problem while
for others a reason to fail the probing.
Do we need a new binging for this, i.e one that would indicate this RTC can (and
should) be able to wake up the board and fail driver probing if this can't be
done?
I'll let Alessandro and Alexander be the judge of that.
Thanks,
Mathieu
> >
> >> +
> >> + rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
> >> + &stm32_rtc_ops, THIS_MODULE);
> >> + if (IS_ERR(rtc->rtc_dev)) {
> >> + ret = PTR_ERR(rtc->rtc_dev);
> >> + dev_err(&pdev->dev, "rtc device registration failed, err=%d\n",
> >> + ret);
> >> + goto err;
> >> + }
> >> +
> >> + /* Handle RTC alarm interrupts */
> >> + ret = devm_request_irq(&pdev->dev, rtc->irq_alarm,
> >> + stm32_rtc_alarm_irq, IRQF_TRIGGER_RISING,
> >> + dev_name(&rtc->rtc_dev->dev), rtc);
> >> + if (ret) {
> >> + dev_err(&pdev->dev, "IRQ%d (alarm interrupt) already claimed\n",
> >> + rtc->irq_alarm);
> >> + goto err;
> >> + }
> >> +
> >> + return 0;
> >> +err:
> >> + clk_disable_unprepare(rtc->ck_rtc);
> >> +
> >> + if (dbp)
> >> + regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP);
> >
> > Same comment as above.
> OK.
> >
> >> +
> >> + device_init_wakeup(&pdev->dev, false);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int __exit stm32_rtc_remove(struct platform_device *pdev)
> >> +{
> >> + struct stm32_rtc *rtc = platform_get_drvdata(pdev);
> >> + unsigned int cr;
> >> +
> >> + /* Disable interrupts */
> >> + stm32_rtc_wpr_unlock(rtc);
> >> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
> >> + cr &= ~STM32_RTC_CR_ALRAIE;
> >> + stm32_rtc_writel(rtc, STM32_RTC_CR, cr);
> >> + stm32_rtc_wpr_lock(rtc);
> >> +
> >> + clk_disable_unprepare(rtc->ck_rtc);
> >> +
> >> + /* Enable backup domain write protection */
> >> + if (dbp)
> >> + regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP);
> >> +
> >> + device_init_wakeup(&pdev->dev, false);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +#ifdef CONFIG_PM_SLEEP
> >> +static int stm32_rtc_suspend(struct device *dev)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> +
> >> + if (device_may_wakeup(dev))
> >> + return enable_irq_wake(rtc->irq_alarm);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int stm32_rtc_resume(struct device *dev)
> >> +{
> >> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
> >> + int ret = 0;
> >> +
> >> + ret = stm32_rtc_wait_sync(rtc);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + if (device_may_wakeup(dev))
> >> + return disable_irq_wake(rtc->irq_alarm);
> >> +
> >> + return ret;
> >> +}
> >> +#endif
> >> +
> >> +static SIMPLE_DEV_PM_OPS(stm32_rtc_pm_ops,
> >> + stm32_rtc_suspend, stm32_rtc_resume);
> >> +
> >> +static struct platform_driver stm32_rtc_driver = {
> >> + .probe = stm32_rtc_probe,
> >> + .remove = stm32_rtc_remove,
> >> + .driver = {
> >> + .name = DRIVER_NAME,
> >> + .pm = &stm32_rtc_pm_ops,
> >> + .of_match_table = stm32_rtc_of_match,
> >> + },
> >> +};
> >> +
> >> +module_platform_driver(stm32_rtc_driver);
> >> +
> >> +MODULE_ALIAS("platform:" DRIVER_NAME);
> >> +MODULE_AUTHOR("Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>");
> >> +MODULE_DESCRIPTION("STMicroelectronics STM32 Real Time Clock driver");
> >> +MODULE_LICENSE("GPL v2");
> >> --
> >> 1.9.1
> >>
>
> Best regards,
> Amelie
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3] arm: dts: zynq: Add MicroZed board support
From: Jagan Teki @ 2016-12-05 16:28 UTC (permalink / raw)
To: Michal Simek
Cc: linux-arm-kernel@lists.infradead.org, devicetree,
linux-kernel@vger.kernel.org, Jagan Teki, Soren Brinkmann
In-Reply-To: <f0c33080-0811-a515-f897-e7f851b13b5e@xilinx.com>
On Mon, Sep 26, 2016 at 8:22 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 23.9.2016 11:48, Jagan Teki wrote:
>> From: Jagan Teki <jteki@openedev.com>
>>
>> Added basic dts support for MicroZed board.
>>
>> - UART
>> - SDHCI
>> - Ethernet
>>
>> Cc: Soren Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> ---
>> Changes for v3:
>> - Add Xilinx copyright
>> Changes for v2:
>> - Add SDHCI
>> - Add Ethernet
>>
>> arch/arm/boot/dts/Makefile | 1 +
>> arch/arm/boot/dts/zynq-microzed.dts | 96 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 97 insertions(+)
>> create mode 100644 arch/arm/boot/dts/zynq-microzed.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index faacd52..4d7b858 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -862,6 +862,7 @@ dtb-$(CONFIG_ARCH_VT8500) += \
>> wm8750-apc8750.dtb \
>> wm8850-w70v2.dtb
>> dtb-$(CONFIG_ARCH_ZYNQ) += \
>> + zynq-microzed.dtb \
>> zynq-parallella.dtb \
>> zynq-zc702.dtb \
>> zynq-zc706.dtb \
>> diff --git a/arch/arm/boot/dts/zynq-microzed.dts b/arch/arm/boot/dts/zynq-microzed.dts
>> new file mode 100644
>> index 0000000..b9376a4
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/zynq-microzed.dts
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright (C) 2011 - 2014 Xilinx
>> + * Copyright (C) 2016 Jagan Teki <jteki@openedev.com>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +/dts-v1/;
>> +/include/ "zynq-7000.dtsi"
>> +
>> +/ {
>> + model = "Zynq MicroZED Development Board";
>> + compatible = "xlnx,zynq-microzed", "xlnx,zynq-7000";
>> +
>> + aliases {
>> + ethernet0 = &gem0;
>> + serial0 = &uart1;
>> + };
>> +
>> + memory {
>> + device_type = "memory";
>> + reg = <0x0 0x40000000>;
>> + };
>> +
>> + chosen {
>> + bootargs = "earlycon";
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + usb_phy0: phy0 {
>> + compatible = "usb-nop-xceiv";
>> + #phy-cells = <0>;
>> + };
>> +};
>> +
>> +&clkc {
>> + ps-clk-frequency = <33333333>;
>> +};
>> +
>> +&gem0 {
>> + status = "okay";
>> + phy-mode = "rgmii-id";
>> + phy-handle = <ðernet_phy>;
>> +
>> + ethernet_phy: ethernet-phy@0 {
>> + reg = <0>;
>> + };
>> +};
>> +
>> +&sdhci0 {
>> + status = "okay";
>> +};
>> +
>> +&uart1 {
>> + status = "okay";
>> +};
>> +
>> +&usb0 {
>> + status = "okay";
>> + dr_mode = "host";
>> + usb-phy = <&usb_phy0>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_usb0_default>;
>> +};
>> +
>> +&pinctrl0 {
>> + pinctrl_usb0_default: usb0-default {
>> + mux {
>> + groups = "usb0_0_grp";
>> + function = "usb0";
>> + };
>> +
>> + conf {
>> + groups = "usb0_0_grp";
>> + slew-rate = <0>;
>> + io-standard = <1>;
>> + };
>> +
>> + conf-rx {
>> + pins = "MIO29", "MIO31", "MIO36";
>> + bias-high-impedance;
>> + };
>> +
>> + conf-tx {
>> + pins = "MIO28", "MIO30", "MIO32", "MIO33", "MIO34",
>> + "MIO35", "MIO37", "MIO38", "MIO39";
>> + bias-disable;
>> + };
>> + };
>> +};
>>
>
> Applied.
Was it missed? I couldn't see it on the source for a while. any help?
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply
* Re: [RFC] New Device Tree property - "bonding"
From: Laurent Pinchart @ 2016-12-05 16:20 UTC (permalink / raw)
To: Rob Herring
Cc: Ramesh Shanmugasundaram,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org,
Chris Paterson, Geert Uytterhoeven,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Maxime Ripard
In-Reply-To: <CAL_JsqJw-KVZswBruG42MUdkmVSEb0L8OWCXbid7b41Ft4EpPQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
On Monday 05 Dec 2016 09:57:32 Rob Herring wrote:
> On Mon, Dec 5, 2016 at 8:40 AM, Laurent Pinchart wrote:
> > On Monday 05 Dec 2016 08:18:34 Rob Herring wrote:
> >> On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram wrote:
> >>> Hello DT maintainers,
> >>>
> >>> In one of the Renesas SoCs we have a device called DRIF (Digital Radio
> >>> Interface) controller. A DRIF channel contains 4 external pins - SCK,
> >>> SYNC, Data pins D0 & D1.
> >>>
> >>> Internally a DRIF channel is made up of two SPI slave devices (also
> >>> called sub-channels here) that share common CLK & SYNC signals but have
> >>> their own resource set. The DRIF channel can have either one of the
> >>> sub-channel active at a time or both. When both sub-channels are active,
> >>> they need to be managed together as one device as they share same CLK &
> >>> SYNC. We plan to tie these two sub-channels together with a new property
> >>> called "renesas,bonding".
> >>
> >> Is there no need to describe the master device? No GPIOs, regulators
> >> or other sideband controls needed? If that's never needed (which seems
> >> doubtful), then I would do something different here probably with the
> >> master device as a child of one DRIF and then phandles to master from
> >> the other DRIFs. Otherwise, this looks fine to me.
> >
> > Here's a bit of background.
> >
> > The DRIF is an SPI receiver. It has three input pins, a clock line, a data
> > line and a sync signal. The device is designed to be connected to a
> > variety of data sources, usually plain SPI (1 data line), IIS (1 data
> > line) but also radio tuners that output I/Q data
> > (http://www.ni.com/tutorial/4805/en/) over two data lines.
> >
> > In the case of IQ each data sample is split in two I and Q values
> > (typically 16 to 20 bits each in this case), and the values are
> > transmitted serially over one data line each. The synchronization and
> > clock signals are common to both data lines. The DRIF is optimized for
> > this use case as the DRIF instances in the SoC (each of them having
> > independent clocks, interrupts and control registers) are grouped by two,
> > and the two instances in a group handle a single data line each but share
> > the same clock and sync input.
> >
> > On the software side we need to group the I and Q values, which are DMA'ed
> > to memory by the two DRIF instances, and make them available to
> > userspace. The V4L2 API used here in SDR (Software Defined Radio) mode
> > supports such use cases and exposes a single device node to userspace
> > that allows control of the two DRIF instances as a single device. To be
> > able to implement this we need kernel code to be aware of DRIF groups
> > and, while binding to the DRIF instances separately, expose only one V4L2
> > device to userspace for each group.
> >
> > There's no master or slave instance from a hardware point of view, but the
> > two instances are not interchangeable as they carry separate information.
> > They must thus be identified at the driver level.
>
> By master, I meant the external master device that generates the IQ
> data, not which of the internal DRIF blocks is a master of the other.
> So back to my question, does the external master device need to be
> described? I worry the answer now for a simple case is no, but then
> later people are going to have cases needing to describe more. We need
> to answer this question first before we can decide what this binding
> should look like.
Oh yes the external device certainly needs to be described. As it is
controlled through a separate, general-purpose I2C or SPI controller, it
should be a child node of that controller. The DRIF handles the data interface
only, not the control interface of the external device.
> > Back to the DT bindings, the need to expose relationships between (mostly)
> > independent devices is quite common nowadays. It has been solved in some
> > cases by creating a separate DT node that does not correspond to any
> > physical hardware and whose sole purpose is to contain phandles to
> > devices that need to be grouped. Drivers then bind to the compatible
> > string of that "virtual" DT node. The proposed bonding property is a
> > different approach to solve a similar problem. Would it be worth it
> > addressing the problem at a more general level and try to design a common
> > solution ?
>
> We already have somewhat standard ways of grouping, but they are per
> type of device (display, sound card, etc.). I'm not sure we gain much
> standardizing across these devices and to some extent that ship has
> sailed. Even within display subsystems, I don't think there is one
> style that fits all. Sometimes a parent subsystem node with children
> makes sense for the h/w. Sometimes that doesn't make sense and we have
> the virtual node with a "ports" property (like sun8i did). I've never
> been too happy with that style largely just because it feels like
> we're letting DRM define the binding. However, it's generally extra
> data that an OS could just ignore. There have also been many display
> bindings that started out with a virtual node and we got rid of it. So
> it's not something I like to encourage and we need to be clear when it
> is okay or not.
>
> To state the problem more generally (at least when using OF graph), I
> think the problem is simply how do we define and group multiple entry
> points to an OF graph. Maybe these should be graph nodes themselves
> rather than phandles to the nodes with the entry points of the graph.
CC'ing Maxime Ripard for the sun8i side.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation
From: Marek Vasut @ 2016-12-05 16:11 UTC (permalink / raw)
To: Boris Brezillon
Cc: Punnaiah Choudary Kalluri, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w, richard-/L3Ra7n9ekc,
cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, michals-gjFFaj9aHVfQT0dZR+AlfA,
kalluripunnaiahchoudary-Re5JQEeQqe8AvxtiuMwx3w,
kpc528-Re5JQEeQqe8AvxtiuMwx3w, Punnaiah Choudary Kalluri
In-Reply-To: <20161205093630.650451d7@bbrezillon>
On 12/05/2016 09:36 AM, Boris Brezillon wrote:
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
>>> This patch adds the dts binding document for arasan nand flash
>>> controller.
>>>
>>> Signed-off-by: Punnaiah Choudary Kalluri <punnaia-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> ---
>>> changes in v6:
>>> - Removed num-cs property
>>> - Separated nandchip from nand controller
>>> changes in v5:
>>> - None
>>> Changes in v4:
>>> - Added num-cs property
>>> - Added clock support
>>> Changes in v3:
>>> - None
>>> Changes in v2:
>>> - None
>>> ---
>>> .../devicetree/bindings/mtd/arasan_nfc.txt | 38 ++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>> new file mode 100644
>>> index 0000000..dcbe7ad
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
>>> @@ -0,0 +1,38 @@
>>> +Arasan Nand Flash Controller with ONFI 3.1 support
>>
>> Arasan NAND Flash ...
>>
>>> +Required properties:
>>> +- compatible: Should be "arasan,nfc-v3p10"
>>
>> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
>> some fallback option which doesn't encode IP version in the compat
>> string ?
>
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific
> compatible strings is a good solution to avoid breaking the DT ABI.
Can we detect it at runtime with this hardware ?
>>
>> Also, shouldn't quirks be handled by DT props instead of effectively
>> encoding them into the compatible string ?
>
> Well, from my experience, it's better to hide as much as possible
> behind the compatible. This way, if new quirks are needed for a
> specific revision, you can update the driver without having to change
> the DT.
That's a good point, yes.
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT
From: Alexandre Torgue @ 2016-12-05 16:09 UTC (permalink / raw)
To: Lee Jones, Benjamin Gaignard
Cc: robh+dt, mark.rutland, devicetree, linux-kernel, thierry.reding,
linux-pwm, jic23, knaack.h, lars, pmeerw, linux-iio,
linux-arm-kernel, fabrice.gasnier, gerald.baeza, arnaud.pouliquen,
linus.walleij, linaro-kernel, Benjamin Gaignard
In-Reply-To: <20161202132251.GL2683@dell>
Hi,
On 12/02/2016 02:22 PM, Lee Jones wrote:
> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>
>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++++++++++++++++++++++-
>> arch/arm/boot/dts/stm32f469-disco.dts | 28 +++
>> 2 files changed, 360 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..8c50d03 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -48,7 +48,7 @@
>> #include "skeleton.dtsi"
>> #include "armv7-m.dtsi"
>> #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
>> -
>> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>> / {
>> clocks {
>> clk_hse: clk-hse {
>> @@ -355,6 +355,21 @@
>> slew-rate = <2>;
>> };
>> };
>> +
>> + pwm1_pins: pwm@1 {
>> + pins {
>> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> + };
>> + };
>> +
>> + pwm3_pins: pwm@3 {
>> + pins {
>> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> + <STM32F429_PB5_FUNC_TIM3_CH2>;
>> + };
>> + };
>> };
>>
>> rcc: rcc@40023810 {
>> @@ -426,6 +441,322 @@
>> interrupts = <80>;
>> clocks = <&rcc 0 38>;
>> };
>> +
>> + gptimer1: gptimer1@40010000 {
>
> timer@xxxxxxx
>
> Node names should be generic and not numbered.
>
> I suggest that this isn't actually a timer either. Is contains a
> timer (and a PWM), but in it's completeness it is not a timer per
> say.
>
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm1@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,breakinput;
>> + st,complementary;
>> + status = "disabled";
>> + };
>> +
>> + timer1@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <27>;
>> + st,input-triggers-names = TIM5_TRGO,
>> + TIM2_TRGO,
>> + TIM4_TRGO,
>> + TIM3_TRGO;
>
> I'm still dubious with matching by strings.
>
> I'll take a look at the C code to see what the alternatives could be.
>
>> + st,output-triggers-names = TIM1_TRGO,
>> + TIM1_CH1,
>> + TIM1_CH2,
>> + TIM1_CH3,
>> + TIM1_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer2: gptimer2@40000000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000000 0x400>;
>> + clocks = <&rcc 0 128>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm2@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,32bits-counter;
>> + status = "disabled";
>> + };
>> +
>> + timer2@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <28>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM8_TRGO,
>> + TIM3_TRGO,
>> + TIM4_TRGO;
>> + st,output-triggers-names = TIM2_TRGO,
>> + TIM2_CH1,
>> + TIM2_CH2,
>> + TIM2_CH3,
>> + TIM2_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer3: gptimer3@40000400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000400 0x400>;
>> + clocks = <&rcc 0 129>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm3@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + status = "disabled";
>> + };
>> +
>> + timer3@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <29>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM8_TRGO,
>> + TIM5_TRGO,
>> + TIM4_TRGO;
>> + st,output-triggers-names = TIM3_TRGO,
>> + TIM3_CH1,
>> + TIM3_CH2,
>> + TIM3_CH3,
>> + TIM3_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer4: gptimer4@40000800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000800 0x400>;
>> + clocks = <&rcc 0 130>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm4@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + status = "disabled";
>> + };
>> +
>> + timer4@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <30>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM2_TRGO,
>> + TIM3_TRGO,
>> + TIM8_TRGO;
>> + st,output-triggers-names = TIM4_TRGO,
>> + TIM4_CH1,
>> + TIM4_CH2,
>> + TIM4_CH3,
>> + TIM4_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer5: gptimer5@40000C00 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000C00 0x400>;
>> + clocks = <&rcc 0 131>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm5@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,32bits-counter;
>> + status = "disabled";
>> + };
>> +
>> + timer5@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <50>;
>> + st,input-triggers-names = TIM2_TRGO,
>> + TIM3_TRGO,
>> + TIM4_TRGO,
>> + TIM8_TRGO;
>> + st,output-triggers-names = TIM5_TRGO,
>> + TIM5_CH1,
>> + TIM5_CH2,
>> + TIM5_CH3,
>> + TIM5_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer6: gptimer6@40001000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001000 0x400>;
>> + clocks = <&rcc 0 132>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer6@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <54>;
>> + st,output-triggers-names = TIM6_TRGO;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer7: gptimer7@40001400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001400 0x400>;
>> + clocks = <&rcc 0 133>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer7@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <55>;
>> + st,output-triggers-names = TIM7_TRGO;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer8: gptimer8@40010400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010400 0x400>;
>> + clocks = <&rcc 0 161>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm8@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,complementary;
>> + st,breakinput;
>> + status = "disabled";
>> + };
>> +
>> + timer8@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <46>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM2_TRGO,
>> + TIM4_TRGO,
>> + TIM5_TRGO;
>> + st,output-triggers-names = TIM8_TRGO,
>> + TIM8_CH1,
>> + TIM8_CH2,
>> + TIM8_CH3,
>> + TIM8_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer9: gptimer9@40014000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014000 0x400>;
>> + clocks = <&rcc 0 176>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm9@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <2>;
>> + status = "disabled";
>> + };
>> +
>> + timer9@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <24>;
>> + st,input-triggers-names = TIM2_TRGO,
>> + TIM3_TRGO;
>> + st,output-triggers-names = TIM9_TRGO,
>> + TIM9_CH1,
>> + TIM9_CH2;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer10: gptimer10@40014400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014400 0x400>;
>> + clocks = <&rcc 0 177>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm10@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer11: gptimer11@40014800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014800 0x400>;
>> + clocks = <&rcc 0 178>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm11@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer12: gptimer12@40001800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001800 0x400>;
>> + clocks = <&rcc 0 134>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm12@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <2>;
>> + status = "disabled";
>> + };
>> +
>> + timer12@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <43>;
>> + st,input-triggers-names = TIM4_TRGO,
>> + TIM5_TRGO;
>> + st,output-triggers-names = TIM12_TRGO,
>> + TIM12_CH1,
>> + TIM12_CH2;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer13: gptimer13@40001C00 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001C00 0x400>;
>> + clocks = <&rcc 0 135>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm13@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer14: gptimer14@40002000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40002000 0x400>;
>> + clocks = <&rcc 0 136>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm14@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> };
>> };
>>
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8a163d7..df4ca7e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -81,3 +81,31 @@
>> &usart3 {
>> status = "okay";
>> };
>> +
>> +&gptimer1 {
>> + status = "okay";
>> +
>> + pwm1@0 {
>> + pinctrl-0 = <&pwm1_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + };
>> +
>> + timer1@0 {
>> + status = "okay";
>> + };
>> +};
>
> This is a much *better* format than before.
>
> I still don't like the '&' syntax though.
Please keep "&" format to match with existing nodes.
>
>> +&gptimer3 {
>> + status = "okay";
>> +
>> + pwm3@0 {
>> + pinctrl-0 = <&pwm3_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + };
>> +
>> + timer3@0 {
>> + status = "okay";
>> + };
>> +};
>
^ permalink raw reply
* Re: [RFC] New Device Tree property - "bonding"
From: Rob Herring @ 2016-12-05 15:57 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Ramesh Shanmugasundaram, frowand.list@gmail.com,
mark.rutland@arm.com, pantelis.antoniou@konsulko.com,
Chris Paterson, Geert Uytterhoeven,
laurent.pinchart+renesas@ideasonboard.com,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <2054258.NRXTrTF0Rj@avalon>
On Mon, Dec 5, 2016 at 8:40 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 05 Dec 2016 08:18:34 Rob Herring wrote:
>> On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram wrote:
>> > Hello DT maintainers,
>> >
>> > In one of the Renesas SoCs we have a device called DRIF (Digital Radio
>> > Interface) controller. A DRIF channel contains 4 external pins - SCK,
>> > SYNC, Data pins D0 & D1.
>> >
>> > Internally a DRIF channel is made up of two SPI slave devices (also called
>> > sub-channels here) that share common CLK & SYNC signals but have their
>> > own resource set. The DRIF channel can have either one of the sub-channel
>> > active at a time or both. When both sub-channels are active, they need to
>> > be managed together as one device as they share same CLK & SYNC. We plan
>> > to tie these two sub-channels together with a new property called
>> > "renesas,bonding".
>>
>> Is there no need to describe the master device? No GPIOs, regulators
>> or other sideband controls needed? If that's never needed (which seems
>> doubtful), then I would do something different here probably with the
>> master device as a child of one DRIF and then phandles to master from
>> the other DRIFs. Otherwise, this looks fine to me.
>
> Here's a bit of background.
>
> The DRIF is an SPI receiver. It has three input pins, a clock line, a data
> line and a sync signal. The device is designed to be connected to a variety of
> data sources, usually plain SPI (1 data line), IIS (1 data line) but also
> radio tuners that output I/Q data (http://www.ni.com/tutorial/4805/en/) over
> two data lines.
>
> In the case of IQ each data sample is split in two I and Q values (typically
> 16 to 20 bits each in this case), and the values are transmitted serially over
> one data line each. The synchronization and clock signals are common to both
> data lines. The DRIF is optimized for this use case as the DRIF instances in
> the SoC (each of them having independent clocks, interrupts and control
> registers) are grouped by two, and the two instances in a group handle a
> single data line each but share the same clock and sync input.
>
> On the software side we need to group the I and Q values, which are DMA'ed to
> memory by the two DRIF instances, and make them available to userspace. The
> V4L2 API used here in SDR (Software Defined Radio) mode supports such use
> cases and exposes a single device node to userspace that allows control of the
> two DRIF instances as a single device. To be able to implement this we need
> kernel code to be aware of DRIF groups and, while binding to the DRIF
> instances separately, expose only one V4L2 device to userspace for each group.
>
> There's no master or slave instance from a hardware point of view, but the two
> instances are not interchangeable as they carry separate information. They
> must thus be identified at the driver level.
By master, I meant the external master device that generates the IQ
data, not which of the internal DRIF blocks is a master of the other.
So back to my question, does the external master device need to be
described? I worry the answer now for a simple case is no, but then
later people are going to have cases needing to describe more. We need
to answer this question first before we can decide what this binding
should look like.
> Back to the DT bindings, the need to expose relationships between (mostly)
> independent devices is quite common nowadays. It has been solved in some cases
> by creating a separate DT node that does not correspond to any physical
> hardware and whose sole purpose is to contain phandles to devices that need to
> be grouped. Drivers then bind to the compatible string of that "virtual" DT
> node. The proposed bonding property is a different approach to solve a similar
> problem. Would it be worth it addressing the problem at a more general level
> and try to design a common solution ?
We already have somewhat standard ways of grouping, but they are per
type of device (display, sound card, etc.). I'm not sure we gain much
standardizing across these devices and to some extent that ship has
sailed. Even within display subsystems, I don't think there is one
style that fits all. Sometimes a parent subsystem node with children
makes sense for the h/w. Sometimes that doesn't make sense and we have
the virtual node with a "ports" property (like sun8i did). I've never
been too happy with that style largely just because it feels like
we're letting DRM define the binding. However, it's generally extra
data that an OS could just ignore. There have also been many display
bindings that started out with a virtual node and we got rid of it. So
it's not something I like to encourage and we need to be clear when it
is okay or not.
To state the problem more generally (at least when using OF graph), I
think the problem is simply how do we define and group multiple entry
points to an OF graph. Maybe these should be graph nodes themselves
rather than phandles to the nodes with the entry points of the graph.
Rob
^ permalink raw reply
* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Tony Lindgren @ 2016-12-05 15:25 UTC (permalink / raw)
To: Tero Kristo
Cc: Michael Turquette, Stephen Boyd,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <ed253013-1f12-9fd8-d007-400a6c6fd427-l0cyMroinI0@public.gmane.org>
* Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [161205 02:09]:
> On 03/12/16 02:18, Tony Lindgren wrote:
> > * Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161202 15:52]:
> > > Quoting Tony Lindgren (2016-12-02 15:12:40)
> > > > * Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161202 14:34]:
> > > > > Quoting Tony Lindgren (2016-10-28 16:54:48)
> > > > > > * Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> [161028 16:37]:
> > > > > > > On 10/28, Tony Lindgren wrote:
> > > > > > > > * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [161028 00:43]:
> > > > > > > > > On 28/10/16 03:50, Stephen Boyd wrote:
> > > > > > > > > > I suppose a PRCM is
> > > > > > > > > > like an MFD that has clocks and resets under it? On other
> > > > > > > > > > platforms we've combined that all into one node and just had
> > > > > > > > > > #clock-cells and #reset-cells in that node. Is there any reason
> > > > > > > > > > we can't do that here?
> > > > > > > > >
> > > > > > > > > For OMAPs, there are typically multiple instances of the PRCM around; OMAP4
> > > > > > > > > for example has:
> > > > > > > > >
> > > > > > > > > cm1 @ 0x4a004000 (clocks + clockdomains)
> > > > > > > > > cm2 @ 0x4a008000 (clocks + clockdomains)
> > > > > > > > > prm @ 0x4a306000 (few clocks + resets + power state handling)
> > > > > > > > > scrm @ 0x4a30a000 (few external clocks + plenty of misc stuff)
> > > > > > > > >
> > > > > > > > > These instances are also under different power/voltage domains which means
> > > > > > > > > their PM behavior is different.
> > > > > > > > >
> > > > > > > > > The idea behind having a clockdomain as a provider was mostly to have the
> > > > > > > > > topology visible : prcm-instance -> clockdomain -> clocks
> > > > > > > >
> > > > > > > > Yeah that's needed to get the interconnect hierarchy right for
> > > > > > > > genpd :)
> > > > > > > >
> > > > > > > > > ... but basically I think it would be possible to drop the clockdomain
> > > > > > > > > representation and just mark the prcm-instance as a clock provider. Tony,
> > > > > > > > > any thoughts on that?
> > > > > > > >
> > > > > > > > No let's not drop the clockdomains as those will be needed when we
> > > > > > > > move things into proper hierarchy within the interconnect instances.
> > > > > > > > This will then help with getting things right with genpd.
> > > > > > > >
> > > > > > > > In the long run we just want to specify clockdomain and the offset of
> > > > > > > > the clock instance within the clockdomain in the dts files.
> > > > > > > >
> > > > > > >
> > > > > > > Sorry, I have very little idea how OMAP hardware works. Do you
> > > > > > > mean that you will have different nodes for each clockdomain so
> > > > > > > that genpd can map 1:1 to the node in dts? But in hardware
> > > > > > > there's a prcm that allows us to control many clock domains
> > > > > > > through register read/writes? How is the interconnect involved?
> > > > > >
> > > > > > There are multiple clockdomains, at least one for each interconnect
> > > > > > instance. Once a clockdomain is idle, the related interconnect can
> > > > > > idle too. So yeah genpd pretty much maps 1:1 with the clockdomains.
> > > > > >
> > > > > > There's more info in for example omap4 TRM section "3.4.1 Device
> > > > > > Power-Management Layout" that shows the voltage/power/clock domains.
> > > > > > The interconnect instances are mostly named there too looking at
> > > > > > the L4/L3 naming.
> > > > >
> > > > > I'm confused on two points:
> > > > >
> > > > > 1) why are the clkdm's acting as clock providers? I've always hated the
> > > > > name "clock domain" since those bits are for managing module state, not
> > > > > clock state. The PRM, CM1 and CM2 provide the clocks, not the
> > > > > clockdomains.
> > > >
> > > > The clock domains have multiple clock inputs that are routed to multiple
> > > > child clocks. So it is a clock :)
> > > >
> > > > See for example omap4430 TRM "3.6.4 CD_WKUP Clock Domain" on page
> > > > 393 in my revision here.
> > > >
> > > > On that page "Figure 3-48" shows CD_WKUP with the four input clocks.
> > > > And then "Table 3-84. CD_WKUP Control and Status Parameters" shows
> > > > the CD_WKUP clock domain specific registers. These registers show
> > > > the status, I think they are all read-only registers. Then CD_WKUP
> > > > has multiple child clocks with configurable registers.
> > > >
> > > > From hardware register point of view, each clock domain has:
> > > >
> > > > - Read-only clockdomain status registers in the beginning of
> > > > the address space
> > > >
> > > > - Multiple similar clock instances register instances each
> > > > mapping to a specific interconnect target module
> > > >
> > > > These are documented in "3.11.16.1 WKUP_CM Register Summary".
> > >
> > > Oh, this is because you are treating the MODULEMODE bits like gate
> > > clocks. I never really figured out if this was the best way to model
> > > those bits since they do more than control a line toggling at a rate.
> > > For instance this bit will affect the master/slave IDLE protocol between
> > > the module and the PRCM.
> >
> > Yes seems like there is some negotiation going on there with the
> > target module. But from practical point of view the CLKCTRL
> > register is the gate for a module functional clock.
>
> There's some confusion on this, clockdomain is effectively a collection of
> clocks, and can be used to force control that collection if needed. Chapter
> "3.1.1.1.3 Clock Domain" in some OMAP4 TRM shows the relationship neatly.
Yeah that's my understanding too.
> > > > From hardware point of view, we ideally want to map interconnect
> > > > target modules to the clock instance offset from the clock domain
> > > > for that interconnect segment. For example gptimer1 clocks would
> > > > be just:
> > > >
> > > > clocks = <&cd_wkup 0x40>;
> > > >
> > > > > 2) why aren't the clock domains modeled as genpds with their associated
> > > > > devices attached to them? Note that it is possible to "nest" genpd
> > > > > objects. This would also allow for the "Clockdomain Dependency"
> > > > > relationships to be properly modeled (see section 3.1.1.1.7 Clock Domain
> > > > > Dependency in the OMAP4 TRM).
> > > >
> > > > Clock domains only route clocks to child clocks. Power domains
> > > > are different registers. The power domains map roughly to
> > > > interconnect instances, there we have registers to disable the
> > > > whole interconnect when idle.
> > >
> > > I'm not talking about power islands at all, but the genpd object in
> > > Linux. For instance, if we treat each clock domain like a clock
> > > provider, how could the functional dependency between clkdm_A and
> > > clkdm_B be asserted?
> >
> > To me it seems that some output of a clockdomain is just a input
> > of another clockdomain? So it's just the usual parent child
> > relationship once we treat a clockdomain just as a clock. Tero
> > probably has some input here.
>
> A clockdomain should be modelled as a genpd, that I agree. However, it
> doesn't prevent it from being a clock provider also, or does it?
>
> > > There is certainly no API for that in the clock framework, but for genpd
> > > your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
> > > against clkdm_B, which would satisfy the requirement. See section
> > > 3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
>
> For static dependencies the apis genpd_add/remove_subdomain could probably
> be used.
>
> > To me it seems the API is just clk_get() :) Do you have some
> > specific example we can use to check? My guess is that the
> > TRM "Clock Domain Dependency" is just the usual parent child
> > relationship between clocks that are the clockdomains..
> >
> > If there is something more magical there certainly that should
> > be considered though.
>
> The hwmods could be transformed to individual genpds also I guess. On DT
> level though, we would still need a clock pointer to the main clock and a
> genpd pointer in addition to that.
Hmm a genpd pointer to where exactly? AFAIK each interconnect
instance should be a genpd provider, and the individual interconnect
target modules should be consumers for that genpd.
> Tony, any thoughts on that? Would this break up the plans for the
> interconnect completely?
Does using genpd for clockdomains cause issues for using genpd for
interconnect instances and the target modules?
The thing I'd be worried about there is that the clockdomains and
their child clocks are just devices sitting on the interconnect,
so we could easily end up with genpd modeling something that does
not represent the hardware.
For example, on 4430 we have:
l4_cfg interconnect
...
segment@0
...
target_module@4000
cm1: cm1@0
...
...
target_module@8000
cm2: cm2@0
...
l4_wkup interonnect
...
segment@0
...
target_module@6000
prm: prm@0
...
target_module@a000
scrm: scrm@0
...
So what do you guys have in mind for using genpd in the above
example for the clockdomains?
To me it seems that the interconnect instances like l4_cfg and
l4_wkup above should be genpd providers. I don't at least yet
follow what we need to do with the clockdomains with genpd :)
Wouldn't just doing clk_get() from one clockdomain clock to
another clockdomain clock (or it's output) be enough to
represent the clockdomain dependencies?
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/2] eeprom: Add IDT 89HPESx driver bindings file
From: Serge Semin @ 2016-12-05 15:25 UTC (permalink / raw)
To: Rob Herring
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
mark.rutland-5wv7dgnIgG8, Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161205144621.yxwr7spmsvyilan5@rob-hp-laptop>
On Mon, Dec 05, 2016 at 08:46:21AM -0600, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Nov 29, 2016 at 01:38:21AM +0300, Serge Semin wrote:
> > See cover-letter for changelog
> >
> > Signed-off-by: Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >
> > ---
> > .../devicetree/bindings/misc/idt_89hpesx.txt | 41 ++++++++++++++++++++++
>
> There's not a better location for this? I can't tell because you don't
> describe what the device is.
>
The device is PCIe-switch EEPROM driver with additional debug-interface to
access the switch CSRs. EEPROM is accesses via a separate i2c-slave
interface of the switch.
There might be another place to put the binding file in. There is a special
location for EEPROM drivers bindings - Documentation/devicetree/bindings/eeprom/ .
But as far as I understood from the files put in there, it's intended for
pure EEPROM drivers only. On the other hand the directory I've chosen:
Documentation/devicetree/bindings/misc/
mostly intended for some unusual devices. My device isn't usual, since it
has CSRs debug-interface as well. Additionally I've found
eeprom-93xx46.txt binding file there, which describes EEPROM bindings.
Anyway if you find the file should be placed in
Documentation/devicetree/bindings/eeprom/ instead, I'll move it, it's not
that a big problem.
> > 1 file changed, 41 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> >
> > diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> > index 0000000..469cc93
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> > @@ -0,0 +1,41 @@
> > +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices
> > +
> > +Required properties:
> > + - compatible : should be "<manufacturer>,<type>"
> > + Basically there is only one manufacturer: idt, but some
> > + compatible devices may be produced in future. Following devices
> > + are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2,
> > + 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2,
> > + 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2;
> > + 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a;
> > + 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2,
> > + 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2,
> > + 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2,
> > + 89hpes64h16ag2;
> > + 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2,
> > + 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5,
> > + 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2,
> > + 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2,
> > + 89hpes48t12, 89hpes48t12g2.
> > + Current implementation of the driver doesn't have any device-
>
> Driver capabilties are irrelevant to bindings.
>
Why? I've told in the comment, that the devices actually differ by the CSRs
map. Even though it's not reflected in the code at the moment, the CSRs
read/write restrictions can be added by some concerned programmer in
future. But If I left something like "compatible : idt,89hpesx" device
only, it will be problematic to add that functionality.
Howbeit If you think it's not necessary and "compatible = idt,89hpesx" is
ok, it's perfectly fine for me to make it this way. The property will be
even simpler, than current approach.
> > + specific functionalities. But since each of them differs
> > + by registers mapping, CSRs read/write restrictions can be
> > + added in future.
> > + - reg : I2C address of the IDT 89HPES device.
> > +
> > +Optional properties:
> > + - read-only : Parameterless property disables writes to the EEPROM
> > + - idt,eesize : Size of EEPROM device connected to IDT 89HPES i2c-master bus
> > + (default value is 4096 bytes if option isn't specified)
> > + - idt,eeaddr : Custom address of EEPROM device
> > + (If not specified IDT 89HPESx device will try to communicate
> > + with EEPROM sited by default address - 0x50)
>
> Don't we already have standard EEPROM properties that could be used
> here?
>
If we do, just tell me which one. There are standard options:
"compatible, reg, pagesize, read-only". There isn't any connected with
EEPROM actual size.
Why so? Because standard EEPROM-drivers determine the device size from the
compatible-string name. Such approach won't work in this case, because
PCIe-switch and it EEPROM are actually two different devices. Look at the
chain of the usual platform board design:
Host <--- i2c ----> i2c-slave iface |PCIe-switch| i2c-master iface <--- i2c ---> EEPROM
As you cas see the Host reaches EEPROM through the set of PCIe-switch
i2c-interfaces. In order to properly get data from it my driver needs actual
EEPROM size and it address in the i2c-master bus of the PCIe-switch, in
addition to the standard reg-field, which is address of PCIe-switch i2c-slave
interface and read-only parameter if EEPROM-device has got WP pin asserted.
> > +
> > +Example:
> > + idt_pcie_sw@60 {
>
> Don't use '_'.
>
Ok, I won't.
> > + compatible = "idt,89hpes12nt3";
> > + reg = <0x60>;
> > + read-only;
> > + idt,eesize = <65536>;
> > + idt,eeaddr = <0x50>;
> > + };
> > --
> > 2.6.6
> >
Waiting for the respond.
Thanks
-Sergey
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Rob Herring @ 2016-12-05 14:51 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Richard Cochran,
Sekhar Nori, linux-kernel, linux-omap, devicetree,
Murali Karicheri, Wingman Kwok
In-Reply-To: <20161128230428.6872-3-grygorii.strashko@ti.com>
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".
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> Documentation/devicetree/bindings/net/keystone-netcp.txt | 2 ++
Please group binding changes into a single patch.
> drivers/net/ethernet/ti/cpts.c | 12 ++++++++++++
> drivers/net/ethernet/ti/cpts.h | 8 +++++++-
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
> index c37b54e..ec4a241 100644
> --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
> +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
> @@ -114,6 +114,8 @@ Optional properties:
> driver to them if needed.
>
> Properties related to cpts configurations.
> + - cpts-rftclk-sel: selects one of up to 32 clocks for time sync
> + reference. Default = 0.
Vendor prefix.
> - cpts_clock_mult/cpts_clock_shift:
> used for converting time counter cycles to ns as in
>
^ permalink raw reply
* Re: [PATCH 1/6] net: ethernet: ti: netcp: add support of cpts
From: Rob Herring @ 2016-12-05 14:49 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Richard Cochran,
Sekhar Nori, linux-kernel, linux-omap, 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:
> From: WingMan Kwok <w-kwok2@ti.com>
>
> This patch adds support of the cpts device found in the
> gbe and 10gbe ethernet switches on the keystone 2 SoCs
> (66AK2E/L/Hx, 66AK2Gx).
>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> .../devicetree/bindings/net/keystone-netcp.txt | 9 +
> drivers/net/ethernet/ti/Kconfig | 7 +-
> drivers/net/ethernet/ti/netcp.h | 2 +-
> drivers/net/ethernet/ti/netcp_core.c | 18 +-
> drivers/net/ethernet/ti/netcp_ethss.c | 437 ++++++++++++++++++++-
> 5 files changed, 459 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
> index 04ba1dc..c37b54e 100644
> --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
> +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
> @@ -113,6 +113,15 @@ Optional properties:
> will only initialize these ports and attach PHY
> driver to them if needed.
>
> + Properties related to cpts configurations.
> + - cpts_clock_mult/cpts_clock_shift:
Needs vendor prefix. Don't use '_'.
> + used for converting time counter cycles to ns as in
> +
> + ns = (cycles * clock_mult) >> _shift
> +
> + Defaults: clock_mult, clock_shift = calculated from
> + CPTS refclk
What does this mean?
> +
> NetCP interface properties: Interface specification for NetCP sub-modules.
> Required properties:
> - rx-channel: the navigator packet dma channel name for rx.
^ permalink raw reply
* Re: [PATCH v2 2/2] eeprom: Add IDT 89HPESx driver bindings file
From: Rob Herring @ 2016-12-05 14:46 UTC (permalink / raw)
To: Serge Semin
Cc: gregkh, srinivas.kandagatla, andrew, mark.rutland, Sergey.Semin,
linux-kernel, devicetree
In-Reply-To: <1480372701-30560-3-git-send-email-fancer.lancer@gmail.com>
On Tue, Nov 29, 2016 at 01:38:21AM +0300, Serge Semin wrote:
> See cover-letter for changelog
>
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
>
> ---
> .../devicetree/bindings/misc/idt_89hpesx.txt | 41 ++++++++++++++++++++++
There's not a better location for this? I can't tell because you don't
describe what the device is.
> 1 file changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
>
> diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> index 0000000..469cc93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> @@ -0,0 +1,41 @@
> +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices
> +
> +Required properties:
> + - compatible : should be "<manufacturer>,<type>"
> + Basically there is only one manufacturer: idt, but some
> + compatible devices may be produced in future. Following devices
> + are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2,
> + 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2,
> + 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2;
> + 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a;
> + 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2,
> + 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2,
> + 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2,
> + 89hpes64h16ag2;
> + 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2,
> + 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5,
> + 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2,
> + 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2,
> + 89hpes48t12, 89hpes48t12g2.
> + Current implementation of the driver doesn't have any device-
Driver capabilties are irrelevant to bindings.
> + specific functionalities. But since each of them differs
> + by registers mapping, CSRs read/write restrictions can be
> + added in future.
> + - reg : I2C address of the IDT 89HPES device.
> +
> +Optional properties:
> + - read-only : Parameterless property disables writes to the EEPROM
> + - idt,eesize : Size of EEPROM device connected to IDT 89HPES i2c-master bus
> + (default value is 4096 bytes if option isn't specified)
> + - idt,eeaddr : Custom address of EEPROM device
> + (If not specified IDT 89HPESx device will try to communicate
> + with EEPROM sited by default address - 0x50)
Don't we already have standard EEPROM properties that could be used
here?
> +
> +Example:
> + idt_pcie_sw@60 {
Don't use '_'.
> + compatible = "idt,89hpes12nt3";
> + reg = <0x60>;
> + read-only;
> + idt,eesize = <65536>;
> + idt,eeaddr = <0x50>;
> + };
> --
> 2.6.6
>
^ permalink raw reply
* Re: [RFC] New Device Tree property - "bonding"
From: Laurent Pinchart @ 2016-12-05 14:40 UTC (permalink / raw)
To: Rob Herring
Cc: Ramesh Shanmugasundaram,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org,
Chris Paterson, Geert Uytterhoeven,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+-6v1z-MYHC8Lvvao+=SFhVn2XwBc3O6fSN_zNcrJ3kA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
On Monday 05 Dec 2016 08:18:34 Rob Herring wrote:
> On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram wrote:
> > Hello DT maintainers,
> >
> > In one of the Renesas SoCs we have a device called DRIF (Digital Radio
> > Interface) controller. A DRIF channel contains 4 external pins - SCK,
> > SYNC, Data pins D0 & D1.
> >
> > Internally a DRIF channel is made up of two SPI slave devices (also called
> > sub-channels here) that share common CLK & SYNC signals but have their
> > own resource set. The DRIF channel can have either one of the sub-channel
> > active at a time or both. When both sub-channels are active, they need to
> > be managed together as one device as they share same CLK & SYNC. We plan
> > to tie these two sub-channels together with a new property called
> > "renesas,bonding".
>
> Is there no need to describe the master device? No GPIOs, regulators
> or other sideband controls needed? If that's never needed (which seems
> doubtful), then I would do something different here probably with the
> master device as a child of one DRIF and then phandles to master from
> the other DRIFs. Otherwise, this looks fine to me.
Here's a bit of background.
The DRIF is an SPI receiver. It has three input pins, a clock line, a data
line and a sync signal. The device is designed to be connected to a variety of
data sources, usually plain SPI (1 data line), IIS (1 data line) but also
radio tuners that output I/Q data (http://www.ni.com/tutorial/4805/en/) over
two data lines.
In the case of IQ each data sample is split in two I and Q values (typically
16 to 20 bits each in this case), and the values are transmitted serially over
one data line each. The synchronization and clock signals are common to both
data lines. The DRIF is optimized for this use case as the DRIF instances in
the SoC (each of them having independent clocks, interrupts and control
registers) are grouped by two, and the two instances in a group handle a
single data line each but share the same clock and sync input.
On the software side we need to group the I and Q values, which are DMA'ed to
memory by the two DRIF instances, and make them available to userspace. The
V4L2 API used here in SDR (Software Defined Radio) mode supports such use
cases and exposes a single device node to userspace that allows control of the
two DRIF instances as a single device. To be able to implement this we need
kernel code to be aware of DRIF groups and, while binding to the DRIF
instances separately, expose only one V4L2 device to userspace for each group.
There's no master or slave instance from a hardware point of view, but the two
instances are not interchangeable as they carry separate information. They
must thus be identified at the driver level.
Back to the DT bindings, the need to expose relationships between (mostly)
independent devices is quite common nowadays. It has been solved in some cases
by creating a separate DT node that does not correspond to any physical
hardware and whose sole purpose is to contain phandles to devices that need to
be grouped. Drivers then bind to the compatible string of that "virtual" DT
node. The proposed bonding property is a different approach to solve a similar
problem. Would it be worth it addressing the problem at a more general level
and try to design a common solution ?
--
Regards,
Laurent Pinchart
--
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 net-next v4 2/4] dt-bindings: net: add EEE capability constants
From: Rob Herring @ 2016-12-05 14:39 UTC (permalink / raw)
To: Jerome Brunet
Cc: netdev, devicetree, Florian Fainelli, Carlo Caione, Kevin Hilman,
Giuseppe Cavallaro, Alexandre TORGUE, Martin Blumenstingl,
Andre Roth, Andrew Lunn, Neil Armstrong, linux-amlogic,
linux-arm-kernel, linux-kernel, Julia Lawall, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1480348229-25672-3-git-send-email-jbrunet@baylibre.com>
On Mon, Nov 28, 2016 at 04:50:26PM +0100, Jerome Brunet wrote:
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
> Tested-by: Andreas Färber <afaerber@suse.de>
> Tested-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 include/dt-bindings/net/mdio.h
Seems changes are wanted on this, but patches 2 and 3 should be
combined. The header is part of the binding doc.
Rob
^ permalink raw reply
* [PATCH v3 3/3] ARM: dts: vf610-zii-dev-rev-b: Remove 'fixed-link' from DSA ports
From: Andrey Smirnov @ 2016-12-05 14:38 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, devicetree, andrew, Andrey Smirnov, Russell King,
Stefan Agner, linux-kernel, Rob Herring, Sascha Hauer, Shawn Guo,
cphealy
In-Reply-To: <1480948716-17744-1-git-send-email-andrew.smirnov@gmail.com>
Remove 'fixed-link' nodes from DSA ports since they are not needed (they
are not limiting link's speed and the ports will be configured to their
maximux speed as a default)
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
Changes since v2:
- None
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 8 --------
1 file changed, 8 deletions(-)
diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index c0fc3f2..646c90c 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -97,10 +97,6 @@
phy-mode = "rgmii-txid";
link = <&switch1port6
&switch2port9>;
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
};
port@6 {
@@ -165,10 +161,6 @@
label = "dsa";
phy-mode = "rgmii-txid";
link = <&switch0port5>;
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
};
};
mdio {
--
2.5.5
^ permalink raw reply related
* [PATCH v3 2/3] ARM: dts: vf610-zii-dev: Add .dts file for rev. C
From: Andrey Smirnov @ 2016-12-05 14:38 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, devicetree, andrew, Andrey Smirnov, Russell King,
Stefan Agner, linux-kernel, Rob Herring, Sascha Hauer, Shawn Guo,
cphealy
In-Reply-To: <1480948716-17744-1-git-send-email-andrew.smirnov@gmail.com>
Add .dts file for rev. C of the board by factoring out commonalities
into a shared include file (vf610-zii-dev-rev-b-c.dtsi) and deriving
revision specific file from it (vf610-zii-dev-rev-b.dts and
vf610-zii-dev-reb-c.dts).
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
Changes since v2:
- Removed unnecessary "ethernet-phy-id0022.1550" string from
ethernet-phy@0 node
- Correctecd IRQ_TYPE_EDGE_FALLING to IRQ_TYPE_LEVEL_LOW in
ethernet-phy@0, since that's what KS8091 corresponding to
that node uses
- Changed PTB28's pinmux configuration to include a 47kOhm
pullup to pull aforementioned PHY IRQ line to unasserted
level by default
arch/arm/boot/dts/Makefile | 3 +-
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 300 +----------------------
arch/arm/boot/dts/vf610-zii-dev-rev-c.dts | 394 ++++++++++++++++++++++++++++++
arch/arm/boot/dts/vf610-zii-dev.dtsi | 383 +++++++++++++++++++++++++++++
4 files changed, 780 insertions(+), 300 deletions(-)
create mode 100644 arch/arm/boot/dts/vf610-zii-dev-rev-c.dts
create mode 100644 arch/arm/boot/dts/vf610-zii-dev.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index befcd26..9f0d2a1 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -442,7 +442,8 @@ dtb-$(CONFIG_SOC_VF610) += \
vf610-cosmic.dtb \
vf610m4-cosmic.dtb \
vf610-twr.dtb \
- vf610-zii-dev-rev-b.dtb
+ vf610-zii-dev-rev-b.dtb \
+ vf610-zii-dev-rev-c.dtb
dtb-$(CONFIG_ARCH_MXS) += \
imx23-evk.dtb \
imx23-olinuxino.dtb \
diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index 2210811..c0fc3f2 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -43,32 +43,12 @@
*/
/dts-v1/;
-#include "vf610.dtsi"
+#include "vf610-zii-dev.dtsi"
/ {
model = "ZII VF610 Development Board, Rev B";
compatible = "zii,vf610dev-b", "zii,vf610dev", "fsl,vf610";
- chosen {
- stdout-path = "serial0:115200n8";
- };
-
- memory {
- reg = <0x80000000 0x20000000>;
- };
-
- gpio-leds {
- compatible = "gpio-leds";
- pinctrl-0 = <&pinctrl_leds_debug>;
- pinctrl-names = "default";
-
- debug {
- label = "zii:green:debug1";
- gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
- linux,default-trigger = "heartbeat";
- };
- };
-
mdio-mux {
compatible = "mdio-mux-gpio";
pinctrl-0 = <&pinctrl_mdio_mux>;
@@ -281,25 +261,6 @@
};
};
- reg_vcc_3v3_mcu: regulator-vcc-3v3-mcu {
- compatible = "regulator-fixed";
- regulator-name = "vcc_3v3_mcu";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- };
-
- usb0_vbus: regulator-usb0-vbus {
- compatible = "regulator-fixed";
- pinctrl-0 = <&pinctrl_usb_vbus>;
- regulator-name = "usb_vbus";
- regulator-min-microvolt = <5000000>;
- regulator-max-microvolt = <5000000>;
- enable-active-high;
- regulator-always-on;
- regulator-boot-on;
- gpio = <&gpio0 6 0>;
- };
-
spi0 {
compatible = "spi-gpio";
pinctrl-0 = <&pinctrl_gpio_spi0>;
@@ -336,49 +297,6 @@
};
};
-&adc0 {
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_adc0_ad5>;
- vref-supply = <®_vcc_3v3_mcu>;
- status = "okay";
-};
-
-&edma0 {
- status = "okay";
-};
-
-&esdhc1 {
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_esdhc1>;
- bus-width = <4>;
- status = "okay";
-};
-
-&fec0 {
- phy-mode = "rmii";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_fec0>;
- status = "okay";
-};
-
-&fec1 {
- phy-mode = "rmii";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_fec1>;
- status = "okay";
-
- fixed-link {
- speed = <100>;
- full-duplex;
- };
-
- mdio1: mdio {
- #address-cells = <1>;
- #size-cells = <0>;
- status = "okay";
- };
-};
-
&i2c0 {
clock-frequency = <100000>;
pinctrl-names = "default";
@@ -403,33 +321,6 @@
interrupt-parent = <&gpio2>;
interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
};
-
- lm75@48 {
- compatible = "national,lm75";
- reg = <0x48>;
- };
-
- at24c04@50 {
- compatible = "atmel,24c04";
- reg = <0x50>;
- };
-
- at24c04@52 {
- compatible = "atmel,24c04";
- reg = <0x52>;
- };
-
- ds1682@6b {
- compatible = "dallas,ds1682";
- reg = <0x6b>;
- };
-};
-
-&i2c1 {
- clock-frequency = <100000>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_i2c1>;
- status = "okay";
};
&i2c2 {
@@ -499,120 +390,8 @@
};
};
-&uart0 {
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_uart0>;
- status = "okay";
-};
-
-&uart1 {
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_uart1>;
- status = "okay";
-};
-
-&uart2 {
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_uart2>;
- status = "okay";
-};
-
-&usbdev0 {
- disable-over-current;
- vbus-supply = <&usb0_vbus>;
- dr_mode = "host";
- status = "okay";
-};
-
-&usbh1 {
- disable-over-current;
- status = "okay";
-};
-
-&usbmisc0 {
- status = "okay";
-};
-
-&usbmisc1 {
- status = "okay";
-};
-
-&usbphy0 {
- status = "okay";
-};
-
-&usbphy1 {
- status = "okay";
-};
&iomuxc {
- pinctrl_adc0_ad5: adc0ad5grp {
- fsl,pins = <
- VF610_PAD_PTC30__ADC0_SE5 0x00a1
- >;
- };
-
- pinctrl_dspi0: dspi0grp {
- fsl,pins = <
- VF610_PAD_PTB18__DSPI0_CS1 0x1182
- VF610_PAD_PTB19__DSPI0_CS0 0x1182
- VF610_PAD_PTB20__DSPI0_SIN 0x1181
- VF610_PAD_PTB21__DSPI0_SOUT 0x1182
- VF610_PAD_PTB22__DSPI0_SCK 0x1182
- >;
- };
-
- pinctrl_dspi2: dspi2grp {
- fsl,pins = <
- VF610_PAD_PTD31__DSPI2_CS1 0x1182
- VF610_PAD_PTD30__DSPI2_CS0 0x1182
- VF610_PAD_PTD29__DSPI2_SIN 0x1181
- VF610_PAD_PTD28__DSPI2_SOUT 0x1182
- VF610_PAD_PTD27__DSPI2_SCK 0x1182
- >;
- };
-
- pinctrl_esdhc1: esdhc1grp {
- fsl,pins = <
- VF610_PAD_PTA24__ESDHC1_CLK 0x31ef
- VF610_PAD_PTA25__ESDHC1_CMD 0x31ef
- VF610_PAD_PTA26__ESDHC1_DAT0 0x31ef
- VF610_PAD_PTA27__ESDHC1_DAT1 0x31ef
- VF610_PAD_PTA28__ESDHC1_DATA2 0x31ef
- VF610_PAD_PTA29__ESDHC1_DAT3 0x31ef
- VF610_PAD_PTA7__GPIO_134 0x219d
- >;
- };
-
- pinctrl_fec0: fec0grp {
- fsl,pins = <
- VF610_PAD_PTC0__ENET_RMII0_MDC 0x30d2
- VF610_PAD_PTC1__ENET_RMII0_MDIO 0x30d3
- VF610_PAD_PTC2__ENET_RMII0_CRS 0x30d1
- VF610_PAD_PTC3__ENET_RMII0_RXD1 0x30d1
- VF610_PAD_PTC4__ENET_RMII0_RXD0 0x30d1
- VF610_PAD_PTC5__ENET_RMII0_RXER 0x30d1
- VF610_PAD_PTC6__ENET_RMII0_TXD1 0x30d2
- VF610_PAD_PTC7__ENET_RMII0_TXD0 0x30d2
- VF610_PAD_PTC8__ENET_RMII0_TXEN 0x30d2
- >;
- };
-
- pinctrl_fec1: fec1grp {
- fsl,pins = <
- VF610_PAD_PTA6__RMII_CLKIN 0x30d1
- VF610_PAD_PTC9__ENET_RMII1_MDC 0x30d2
- VF610_PAD_PTC10__ENET_RMII1_MDIO 0x30d3
- VF610_PAD_PTC11__ENET_RMII1_CRS 0x30d1
- VF610_PAD_PTC12__ENET_RMII1_RXD1 0x30d1
- VF610_PAD_PTC13__ENET_RMII1_RXD0 0x30d1
- VF610_PAD_PTC14__ENET_RMII1_RXER 0x30d1
- VF610_PAD_PTC15__ENET_RMII1_TXD1 0x30d2
- VF610_PAD_PTC16__ENET_RMII1_TXD0 0x30d2
- VF610_PAD_PTC17__ENET_RMII1_TXEN 0x30d2
- >;
- };
-
pinctrl_gpio_e6185_eeprom_sel: pinctrl-gpio-e6185-eeprom-spi0 {
fsl,pins = <
VF610_PAD_PTE27__GPIO_132 0x33e2
@@ -629,39 +408,6 @@
>;
};
- pinctrl_i2c_mux_reset: pinctrl-i2c-mux-reset {
- fsl,pins = <
- VF610_PAD_PTE14__GPIO_119 0x31c2
- >;
- };
-
- pinctrl_i2c0: i2c0grp {
- fsl,pins = <
- VF610_PAD_PTB14__I2C0_SCL 0x37ff
- VF610_PAD_PTB15__I2C0_SDA 0x37ff
- >;
- };
-
- pinctrl_i2c1: i2c1grp {
- fsl,pins = <
- VF610_PAD_PTB16__I2C1_SCL 0x37ff
- VF610_PAD_PTB17__I2C1_SDA 0x37ff
- >;
- };
-
- pinctrl_i2c2: i2c2grp {
- fsl,pins = <
- VF610_PAD_PTA22__I2C2_SCL 0x37ff
- VF610_PAD_PTA23__I2C2_SDA 0x37ff
- >;
- };
-
- pinctrl_leds_debug: pinctrl-leds-debug {
- fsl,pins = <
- VF610_PAD_PTD20__GPIO_74 0x31c2
- >;
- };
-
pinctrl_mdio_mux: pinctrl-mdio-mux {
fsl,pins = <
VF610_PAD_PTA18__GPIO_8 0x31c2
@@ -676,48 +422,4 @@
VF610_PAD_PTB28__GPIO_98 0x219d
>;
};
-
- pinctrl_qspi0: qspi0grp {
- fsl,pins = <
- VF610_PAD_PTD7__QSPI0_B_QSCK 0x31c3
- VF610_PAD_PTD8__QSPI0_B_CS0 0x31ff
- VF610_PAD_PTD9__QSPI0_B_DATA3 0x31c3
- VF610_PAD_PTD10__QSPI0_B_DATA2 0x31c3
- VF610_PAD_PTD11__QSPI0_B_DATA1 0x31c3
- VF610_PAD_PTD12__QSPI0_B_DATA0 0x31c3
- >;
- };
-
- pinctrl_uart0: uart0grp {
- fsl,pins = <
- VF610_PAD_PTB10__UART0_TX 0x21a2
- VF610_PAD_PTB11__UART0_RX 0x21a1
- >;
- };
-
- pinctrl_uart1: uart1grp {
- fsl,pins = <
- VF610_PAD_PTB23__UART1_TX 0x21a2
- VF610_PAD_PTB24__UART1_RX 0x21a1
- >;
- };
-
- pinctrl_uart2: uart2grp {
- fsl,pins = <
- VF610_PAD_PTD0__UART2_TX 0x21a2
- VF610_PAD_PTD1__UART2_RX 0x21a1
- >;
- };
-
- pinctrl_usb_vbus: pinctrl-usb-vbus {
- fsl,pins = <
- VF610_PAD_PTA16__GPIO_6 0x31c2
- >;
- };
-
- pinctrl_usb0_host: usb0-host-grp {
- fsl,pins = <
- VF610_PAD_PTD6__GPIO_85 0x0062
- >;
- };
};
diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts
new file mode 100644
index 0000000..4d38f41
--- /dev/null
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts
@@ -0,0 +1,394 @@
+/*
+ * Copyright (C) 2015, 2016 Zodiac Inflight Innovations
+ *
+ * Based on an original 'vf610-twr.dts' which is Copyright 2015,
+ * Freescale Semiconductor, Inc.
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "vf610-zii-dev.dtsi"
+
+/ {
+ model = "ZII VF610 Development Board, Rev C";
+ compatible = "zii,vf610dev-c", "zii,vf610dev", "fsl,vf610";
+
+ mdio-mux {
+ compatible = "mdio-mux-gpio";
+ pinctrl-0 = <&pinctrl_mdio_mux>;
+ pinctrl-names = "default";
+ gpios = <&gpio0 8 GPIO_ACTIVE_HIGH
+ &gpio0 9 GPIO_ACTIVE_HIGH
+ &gpio0 25 GPIO_ACTIVE_HIGH>;
+ mdio-parent-bus = <&mdio1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mdio_mux_1: mdio@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch0: switch0@0 {
+ compatible = "marvell,mv88e6190";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ dsa,member = <0 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "cpu";
+ ethernet = <&fec1>;
+ fixed-link {
+ speed = <100>;
+ full-duplex;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan1";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan2";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan3";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "lan4";
+ };
+
+ switch0port10: port@10 {
+ reg = <10>;
+ label = "dsa";
+ phy-mode = "xgmii";
+ link = <&switch1port10>;
+ };
+ };
+ };
+ };
+
+ mdio_mux_2: mdio@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch1: switch1@0 {
+ compatible = "marvell,mv88e6190";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ dsa,member = <0 1>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+ label = "lan5";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan6";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan7";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "lan8";
+ };
+
+
+ switch1port10: port@10 {
+ reg = <10>;
+ label = "dsa";
+ phy-mode = "xgmii";
+ link = <&switch0port10>;
+ };
+ };
+ };
+ };
+
+ mdio_mux_4: mdio@4 {
+ reg = <4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+};
+
+&dspi0 {
+ bus-num = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_dspi0>;
+ status = "okay";
+ spi-num-chipselects = <2>;
+
+ m25p128@0 {
+ compatible = "m25p128", "jedec,spi-nor";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ };
+};
+
+&i2c0 {
+ /*
+ * U712
+ *
+ * Exposed signals:
+ * P1 - WE2_CMD
+ * P2 - WE2_CLK
+ */
+ gpio5: pca9557@18 {
+ compatible = "nxp,pca9557";
+ reg = <0x18>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ /*
+ * U121
+ *
+ * Exposed signals:
+ * I/O0 - ENET_SWR_EN
+ * I/O1 - ESW1_RESETn
+ * I/O2 - ARINC_RESET
+ * I/O3 - DD1_IO_RESET
+ * I/O4 - ESW2_RESETn
+ * I/O5 - ESW3_RESETn
+ * I/O6 - ESW4_RESETn
+ * I/O8 - TP909
+ * I/O9 - FEM_SEL
+ * I/O10 - WIFI_RESETn
+ * I/O11 - PHY_RSTn
+ * I/O12 - OPT1_SD
+ * I/O13 - OPT2_SD
+ * I/O14 - OPT1_TX_DIS
+ * I/O15 - OPT2_TX_DIS
+ */
+ gpio6: sx1503@20 {
+ compatible = "semtech,sx1503q";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sx1503_20>;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ reg = <0x20>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+ gpio-controller;
+ interrupt-controller;
+
+ enet_swr_en {
+ gpio-hog;
+ gpios = <0 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "enet-swr-en";
+ };
+ };
+
+ /*
+ * U715
+ *
+ * Exposed signals:
+ * IO0 - WE1_CLK
+ * IO1 - WE1_CMD
+ */
+ gpio7: pca9554@22 {
+ compatible = "nxp,pca9554";
+ reg = <0x22>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ };
+};
+
+&i2c1 {
+ at24mac602@00 {
+ compatible = "atmel,24c02";
+ reg = <0x50>;
+ read-only;
+ };
+};
+
+&i2c2 {
+ tca9548@70 {
+ compatible = "nxp,pca9548";
+ pinctrl-0 = <&pinctrl_i2c_mux_reset>;
+ pinctrl-names = "default";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ reset-gpios = <&gpio3 23 GPIO_ACTIVE_LOW>;
+
+ i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+
+ i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ sfp2: at24c04@50 {
+ compatible = "atmel,24c02";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+
+ sfp3: at24c04@50 {
+ compatible = "atmel,24c02";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ };
+ };
+};
+
+&uart3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart3>;
+ status = "okay";
+};
+
+&gpio0 {
+ eth0_intrp {
+ gpio-hog;
+ gpios = <23 GPIO_ACTIVE_HIGH>;
+ input;
+ line-name = "sx1503-irq";
+ };
+};
+
+&gpio3 {
+ eth0_intrp {
+ gpio-hog;
+ gpios = <2 GPIO_ACTIVE_HIGH>;
+ input;
+ line-name = "eth0-intrp";
+ };
+};
+
+&fec0 {
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_fec0_phy_int>;
+
+ interrupt-parent = <&gpio3>;
+ interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+ reg = <0>;
+ };
+ };
+};
+
+&iomuxc {
+ pinctr_atzb_rf_233: pinctrl-atzb-rf-233 {
+ fsl,pins = <
+ VF610_PAD_PTB2__GPIO_24 0x31c2
+ VF610_PAD_PTE27__GPIO_132 0x33e2
+ >;
+ };
+
+
+ pinctrl_sx1503_20: pinctrl-sx1503-20 {
+ fsl,pins = <
+ VF610_PAD_PTB1__GPIO_23 0x219d
+ >;
+ };
+
+ pinctrl_uart3: uart3grp {
+ fsl,pins = <
+ VF610_PAD_PTA20__UART3_TX 0x21a2
+ VF610_PAD_PTA21__UART3_RX 0x21a1
+ >;
+ };
+
+ pinctrl_mdio_mux: pinctrl-mdio-mux {
+ fsl,pins = <
+ VF610_PAD_PTA18__GPIO_8 0x31c2
+ VF610_PAD_PTA19__GPIO_9 0x31c2
+ VF610_PAD_PTB3__GPIO_25 0x31c2
+ >;
+ };
+
+ pinctrl_fec0_phy_int: pinctrl-fec0-phy-int {
+ fsl,pins = <
+ VF610_PAD_PTB28__GPIO_98 0x219d
+ >;
+ };
+};
diff --git a/arch/arm/boot/dts/vf610-zii-dev.dtsi b/arch/arm/boot/dts/vf610-zii-dev.dtsi
new file mode 100644
index 0000000..9f5e2e7
--- /dev/null
+++ b/arch/arm/boot/dts/vf610-zii-dev.dtsi
@@ -0,0 +1,383 @@
+/*
+ * Copyright (C) 2015, 2016 Zodiac Inflight Innovations
+ *
+ * Based on an original 'vf610-twr.dts' which is Copyright 2015,
+ * Freescale Semiconductor, Inc.
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use
+n * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include "vf610.dtsi"
+
+/ {
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ reg = <0x80000000 0x20000000>;
+ };
+
+ gpio-leds {
+ compatible = "gpio-leds";
+ pinctrl-0 = <&pinctrl_leds_debug>;
+ pinctrl-names = "default";
+
+ debug {
+ label = "zii:green:debug1";
+ gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+
+ reg_vcc_3v3_mcu: regulator-vcc-3v3-mcu {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc_3v3_mcu";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ usb0_vbus: regulator-usb0-vbus {
+ compatible = "regulator-fixed";
+ pinctrl-0 = <&pinctrl_usb_vbus>;
+ regulator-name = "usb_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ regulator-always-on;
+ regulator-boot-on;
+ gpio = <&gpio0 6 0>;
+ };
+};
+
+&adc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_adc0_ad5>;
+ vref-supply = <®_vcc_3v3_mcu>;
+ status = "okay";
+};
+
+&edma0 {
+ status = "okay";
+};
+
+&esdhc1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_esdhc1>;
+ bus-width = <4>;
+ status = "okay";
+};
+
+&fec0 {
+ phy-mode = "rmii";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_fec0>;
+ status = "okay";
+};
+
+&fec1 {
+ phy-mode = "rmii";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_fec1>;
+ status = "okay";
+
+ fixed-link {
+ speed = <100>;
+ full-duplex;
+ };
+
+ mdio1: mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+ };
+};
+
+&i2c0 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default", "gpio";
+ pinctrl-0 = <&pinctrl_i2c0>;
+ pinctrl-1 = <&pinctrl_i2c0_gpio>;
+ scl-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ sda-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+ status = "okay";
+
+ lm75@48 {
+ compatible = "national,lm75";
+ reg = <0x48>;
+ };
+
+ at24c04@50 {
+ compatible = "atmel,24c04";
+ reg = <0x50>;
+ };
+
+ at24c04@52 {
+ compatible = "atmel,24c04";
+ reg = <0x52>;
+ };
+
+ ds1682@6b {
+ compatible = "dallas,ds1682";
+ reg = <0x6b>;
+ };
+};
+
+&i2c1 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c1>;
+ status = "okay";
+};
+
+&i2c2 {
+ clock-frequency = <100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c2>;
+ status = "okay";
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart0>;
+ status = "okay";
+};
+
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart1>;
+ status = "okay";
+};
+
+&uart2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart2>;
+ status = "okay";
+};
+
+&usbdev0 {
+ disable-over-current;
+ vbus-supply = <&usb0_vbus>;
+ dr_mode = "host";
+ status = "okay";
+};
+
+&usbh1 {
+ disable-over-current;
+ status = "okay";
+};
+
+&usbmisc0 {
+ status = "okay";
+};
+
+&usbmisc1 {
+ status = "okay";
+};
+
+&usbphy0 {
+ status = "okay";
+};
+
+&usbphy1 {
+ status = "okay";
+};
+
+&iomuxc {
+ pinctrl_adc0_ad5: adc0ad5grp {
+ fsl,pins = <
+ VF610_PAD_PTC30__ADC0_SE5 0x00a1
+ >;
+ };
+
+ pinctrl_dspi0: dspi0grp {
+ fsl,pins = <
+ VF610_PAD_PTB18__DSPI0_CS1 0x1182
+ VF610_PAD_PTB19__DSPI0_CS0 0x1182
+ VF610_PAD_PTB20__DSPI0_SIN 0x1181
+ VF610_PAD_PTB21__DSPI0_SOUT 0x1182
+ VF610_PAD_PTB22__DSPI0_SCK 0x1182
+ >;
+ };
+
+ pinctrl_dspi2: dspi2grp {
+ fsl,pins = <
+ VF610_PAD_PTD31__DSPI2_CS1 0x1182
+ VF610_PAD_PTD30__DSPI2_CS0 0x1182
+ VF610_PAD_PTD29__DSPI2_SIN 0x1181
+ VF610_PAD_PTD28__DSPI2_SOUT 0x1182
+ VF610_PAD_PTD27__DSPI2_SCK 0x1182
+ >;
+ };
+
+ pinctrl_esdhc1: esdhc1grp {
+ fsl,pins = <
+ VF610_PAD_PTA24__ESDHC1_CLK 0x31ef
+ VF610_PAD_PTA25__ESDHC1_CMD 0x31ef
+ VF610_PAD_PTA26__ESDHC1_DAT0 0x31ef
+ VF610_PAD_PTA27__ESDHC1_DAT1 0x31ef
+ VF610_PAD_PTA28__ESDHC1_DATA2 0x31ef
+ VF610_PAD_PTA29__ESDHC1_DAT3 0x31ef
+ VF610_PAD_PTA7__GPIO_134 0x219d
+ >;
+ };
+
+ pinctrl_fec0: fec0grp {
+ fsl,pins = <
+ VF610_PAD_PTC0__ENET_RMII0_MDC 0x30d2
+ VF610_PAD_PTC1__ENET_RMII0_MDIO 0x30d3
+ VF610_PAD_PTC2__ENET_RMII0_CRS 0x30d1
+ VF610_PAD_PTC3__ENET_RMII0_RXD1 0x30d1
+ VF610_PAD_PTC4__ENET_RMII0_RXD0 0x30d1
+ VF610_PAD_PTC5__ENET_RMII0_RXER 0x30d1
+ VF610_PAD_PTC6__ENET_RMII0_TXD1 0x30d2
+ VF610_PAD_PTC7__ENET_RMII0_TXD0 0x30d2
+ VF610_PAD_PTC8__ENET_RMII0_TXEN 0x30d2
+ >;
+ };
+
+ pinctrl_fec1: fec1grp {
+ fsl,pins = <
+ VF610_PAD_PTA6__RMII_CLKIN 0x30d1
+ VF610_PAD_PTC9__ENET_RMII1_MDC 0x30d2
+ VF610_PAD_PTC10__ENET_RMII1_MDIO 0x30d3
+ VF610_PAD_PTC11__ENET_RMII1_CRS 0x30d1
+ VF610_PAD_PTC12__ENET_RMII1_RXD1 0x30d1
+ VF610_PAD_PTC13__ENET_RMII1_RXD0 0x30d1
+ VF610_PAD_PTC14__ENET_RMII1_RXER 0x30d1
+ VF610_PAD_PTC15__ENET_RMII1_TXD1 0x30d2
+ VF610_PAD_PTC16__ENET_RMII1_TXD0 0x30d2
+ VF610_PAD_PTC17__ENET_RMII1_TXEN 0x30d2
+ >;
+ };
+
+ pinctrl_gpio_spi0: pinctrl-gpio-spi0 {
+ fsl,pins = <
+ VF610_PAD_PTB22__GPIO_44 0x33e2
+ VF610_PAD_PTB21__GPIO_43 0x33e2
+ VF610_PAD_PTB20__GPIO_42 0x33e1
+ VF610_PAD_PTB19__GPIO_41 0x33e2
+ VF610_PAD_PTB18__GPIO_40 0x33e2
+ >;
+ };
+
+ pinctrl_i2c_mux_reset: pinctrl-i2c-mux-reset {
+ fsl,pins = <
+ VF610_PAD_PTE14__GPIO_119 0x31c2
+ >;
+ };
+
+ pinctrl_i2c0: i2c0grp {
+ fsl,pins = <
+ VF610_PAD_PTB14__I2C0_SCL 0x37ff
+ VF610_PAD_PTB15__I2C0_SDA 0x37ff
+ >;
+ };
+
+ pinctrl_i2c0_gpio: i2c0grp-gpio {
+ fsl,pins = <
+ VF610_PAD_PTB14__GPIO_36 0x31c2
+ VF610_PAD_PTB15__GPIO_37 0x31c2
+ >;
+ };
+
+
+ pinctrl_i2c1: i2c1grp {
+ fsl,pins = <
+ VF610_PAD_PTB16__I2C1_SCL 0x37ff
+ VF610_PAD_PTB17__I2C1_SDA 0x37ff
+ >;
+ };
+
+ pinctrl_i2c2: i2c2grp {
+ fsl,pins = <
+ VF610_PAD_PTA22__I2C2_SCL 0x37ff
+ VF610_PAD_PTA23__I2C2_SDA 0x37ff
+ >;
+ };
+
+ pinctrl_leds_debug: pinctrl-leds-debug {
+ fsl,pins = <
+ VF610_PAD_PTD20__GPIO_74 0x31c2
+ >;
+ };
+
+ pinctrl_qspi0: qspi0grp {
+ fsl,pins = <
+ VF610_PAD_PTD7__QSPI0_B_QSCK 0x31c3
+ VF610_PAD_PTD8__QSPI0_B_CS0 0x31ff
+ VF610_PAD_PTD9__QSPI0_B_DATA3 0x31c3
+ VF610_PAD_PTD10__QSPI0_B_DATA2 0x31c3
+ VF610_PAD_PTD11__QSPI0_B_DATA1 0x31c3
+ VF610_PAD_PTD12__QSPI0_B_DATA0 0x31c3
+ >;
+ };
+
+ pinctrl_uart0: uart0grp {
+ fsl,pins = <
+ VF610_PAD_PTB10__UART0_TX 0x21a2
+ VF610_PAD_PTB11__UART0_RX 0x21a1
+ >;
+ };
+
+ pinctrl_uart1: uart1grp {
+ fsl,pins = <
+ VF610_PAD_PTB23__UART1_TX 0x21a2
+ VF610_PAD_PTB24__UART1_RX 0x21a1
+ >;
+ };
+
+ pinctrl_uart2: uart2grp {
+ fsl,pins = <
+ VF610_PAD_PTD0__UART2_TX 0x21a2
+ VF610_PAD_PTD1__UART2_RX 0x21a1
+ >;
+ };
+
+ pinctrl_usb_vbus: pinctrl-usb-vbus {
+ fsl,pins = <
+ VF610_PAD_PTA16__GPIO_6 0x31c2
+ >;
+ };
+
+ pinctrl_usb0_host: usb0-host-grp {
+ fsl,pins = <
+ VF610_PAD_PTD6__GPIO_85 0x0062
+ >;
+ };
+};
--
2.5.5
^ permalink raw reply related
* [PATCH v3 1/3] ARM: dts: vf610-zii-dev-rev-b: Remove leftover PWM pingroup
From: Andrey Smirnov @ 2016-12-05 14:38 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, devicetree, andrew, Andrey Smirnov, Russell King,
Stefan Agner, linux-kernel, Rob Herring, Sascha Hauer, Shawn Guo,
cphealy
Remove pwm0grp since it is:
a) Not referenced anywhere in the DTS file (unlike Tower board it
is based on, this board does not use/expose FTM0)
b) Configures PTB2 and PTB3 in a way that contradicts
pinctrl-mdio-mux
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
Changes since v2:
- None
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index fa19cfd..2210811 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -677,15 +677,6 @@
>;
};
- pinctrl_pwm0: pwm0grp {
- fsl,pins = <
- VF610_PAD_PTB0__FTM0_CH0 0x1582
- VF610_PAD_PTB1__FTM0_CH1 0x1582
- VF610_PAD_PTB2__FTM0_CH2 0x1582
- VF610_PAD_PTB3__FTM0_CH3 0x1582
- >;
- };
-
pinctrl_qspi0: qspi0grp {
fsl,pins = <
VF610_PAD_PTD7__QSPI0_B_QSCK 0x31c3
--
2.5.5
^ permalink raw reply related
* Re: [PATCH v3 1/2] ARM: dts: da850-lcdk: add the dumb-vga-dac node
From: Rob Herring @ 2016-12-05 14:32 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Mark Rutland, linux-devicetree, Kevin Hilman, Michael Turquette,
Sekhar Nori, Russell King, linux-drm, LKML, Peter Ujfalusi,
Tomi Valkeinen, Jyri Sarha, Frank Rowand, arm-soc,
Laurent Pinchart
In-Reply-To: <1480420624-23544-2-git-send-email-bgolaszewski@baylibre.com>
On Tue, Nov 29, 2016 at 5:57 AM, Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> Add the dumb-vga-dac node to the board DT together with corresponding
> ports and vga connector. This allows to retrieve the edid info from
> the display automatically.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> arch/arm/boot/dts/da850-lcdk.dts | 58 ++++++++++++++++++++++++++++++++++++++++
> arch/arm/boot/dts/da850.dtsi | 17 ++++++++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index 711b9ad..d864f11 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -50,6 +50,53 @@
> system-clock-frequency = <24576000>;
> };
> };
> +
> + vga_bridge {
s/_/-/
> + compatible = "dumb-vga-dac";
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcd_pins>;
Are these pins from the LCD controller? They should be part of the LCD
controller node.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2] of/irq: improve error report on irq discovery process failure
From: Rob Herring @ 2016-12-05 14:28 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linuxppc-dev, Mark Rutland, Benjamin Herrenschmidt, Frank Rowand,
Marc Zyngier
In-Reply-To: <1480946356-16778-1-git-send-email-gpiccoli@linux.vnet.ibm.com>
On Mon, Dec 5, 2016 at 7:59 AM, Guilherme G. Piccoli
<gpiccoli@linux.vnet.ibm.com> wrote:
> On PowerPC machines some PCI slots might not have level triggered
> interrupts capability (also know as level signaled interrupts),
> leading of_irq_parse_pci() to complain by presenting error messages
> on the kernel log - in this case, the properties "interrupt-map" and
> "interrupt-map-mask" are not present on device's node in the device
> tree.
>
> This patch introduces a different message for this specific case,
> and also reduces its level from error to warning. Besides, we warn
> (once) that possibly some PCI slots on the system have no level
> triggered interrupts available.
> We changed some error return codes too on function of_irq_parse_raw()
> in order other failure's cases can be presented in a more precise way.
>
> Before this patch, when an adapter was plugged in a slot without level
> interrupts capabilitiy on PowerPC, we saw a generic error message
> like this:
>
> [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22
>
> Now, with this applied, we see the following specific message:
>
> [16.154] pci 0014:60:00.1: of_irq_parse_pci: no interrupt-map found,
> INTx interrupts not available
>
> Finally, we standardize the error path in of_irq_parse_raw() by always
> taking the fail path instead of returning directly from the loop.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
>
> v2:
> * Changed function return code to always return negative values;
Are you sure this is safe? This is tricky because of differing values
of NO_IRQ (0 or -1).
> * Improved/simplified warning outputs;
> * Changed some return codes and some error paths in of_irq_parse_raw()
> in order to be more precise/consistent;
This too could have some side effects on callers.
Not saying don't do these changes, just need some assurances this has
been considered.
Rob
^ permalink raw reply
* Re: [PATCH V7 00/10] PM / OPP: Multiple regulator support
From: Viresh Kumar @ 2016-12-05 14:21 UTC (permalink / raw)
To: Rafael Wysocki
Cc: Lists linaro-kernel, linux-pm@vger.kernel.org, Stephen Boyd,
Nishanth Menon, Vincent Guittot, Rob Herring, Dave Gerlach,
Mark Brown, devicetree@vger.kernel.org, Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar@linaro.org>
On 1 December 2016 at 16:28, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi,
>
> Some platforms (like TI) have complex DVFS configuration for CPU
> devices, where multiple regulators are required to be configured to
> change DVFS state of the device. This was explained well by Nishanth
> earlier [1].
>
> One of the major complaints around multiple regulators case was that the
> DT isn't responsible in any way to represent the order in which multiple
> supplies need to be programmed, before or after a frequency change. It
> was considered in this patch and such information is left for the
> platform specific OPP driver now, which can register its own
> opp_set_rate() callback with the OPP core and the OPP core will then
> call it during DVFS.
>
> The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
> and code to pass values for multiple regulators and verified that they
> are all properly read by the kernel (using debugfs interface).
>
> Dave Gerlach has already tested [2] it on the real TI platforms and it
> works well for him.
>
> This is rebased over: linux-next branch in the PM tree.
>
> V6->V7:
> - Added RBY from Stephen for the 8th patch as well.
> - Rebased over pm/bleeding-edge as the dependency patch is already
> applied there.
> - s/dev_pm_opp_set_regulator()/dev_pm_opp_set_regulators() in a comment.
> - Removed the local 'names' array in cpufreq-dt and used "&name"
> instead.
> - Only the 6th patch doesn't have a Reviewed-by Tag now..
Hi Rafael,
V6 of this series received some minor comments [1] and I have
resolved them all here. Now that the merge window is so close,
I think we should be merge it now so that it gets a chance to live
in linux-next for few days.
--
viresh
[1] https://marc.info/?l=linux-pm&m=148054543630253&w=2
^ permalink raw reply
* Re: [RFC] New Device Tree property - "bonding"
From: Rob Herring @ 2016-12-05 14:18 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org,
Chris Paterson, Geert Uytterhoeven,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <KL1PR06MB1031872E5DC2C65F2A490D6BC3890-k6wCOA2IOKSdmgae6ZAqr20DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org> wrote:
> Hello DT maintainers,
>
> In one of the Renesas SoCs we have a device called DRIF (Digital Radio Interface) controller. A DRIF channel contains 4 external pins - SCK, SYNC, Data pins D0 & D1.
>
> Internally a DRIF channel is made up of two SPI slave devices (also called sub-channels here) that share common CLK & SYNC signals but have their own resource set. The DRIF channel can have either one of the sub-channel active at a time or both. When both sub-channels are active, they need to be managed together as one device as they share same CLK & SYNC. We plan to tie these two sub-channels together with a new property called "renesas,bonding".
Is there no need to describe the master device? No GPIOs, regulators
or other sideband controls needed? If that's never needed (which seems
doubtful), then I would do something different here probably with the
master device as a child of one DRIF and then phandles to master from
the other DRIFs. Otherwise, this looks fine to me.
Rob
--
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] of/irq: improve error report on irq discovery process failure
From: Guilherme G. Piccoli @ 2016-12-05 13:59 UTC (permalink / raw)
To: linux-pci, devicetree, linuxppc-dev
Cc: mark.rutland, gpiccoli, marc.zyngier, robh+dt, frowand.list
On PowerPC machines some PCI slots might not have level triggered
interrupts capability (also know as level signaled interrupts),
leading of_irq_parse_pci() to complain by presenting error messages
on the kernel log - in this case, the properties "interrupt-map" and
"interrupt-map-mask" are not present on device's node in the device
tree.
This patch introduces a different message for this specific case,
and also reduces its level from error to warning. Besides, we warn
(once) that possibly some PCI slots on the system have no level
triggered interrupts available.
We changed some error return codes too on function of_irq_parse_raw()
in order other failure's cases can be presented in a more precise way.
Before this patch, when an adapter was plugged in a slot without level
interrupts capabilitiy on PowerPC, we saw a generic error message
like this:
[54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22
Now, with this applied, we see the following specific message:
[16.154] pci 0014:60:00.1: of_irq_parse_pci: no interrupt-map found,
INTx interrupts not available
Finally, we standardize the error path in of_irq_parse_raw() by always
taking the fail path instead of returning directly from the loop.
Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
v2:
* Changed function return code to always return negative values;
* Improved/simplified warning outputs;
* Changed some return codes and some error paths in of_irq_parse_raw()
in order to be more precise/consistent;
drivers/of/irq.c | 19 ++++++++++++-------
drivers/of/of_pci_irq.c | 10 +++++++++-
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 393fea8..9deee86 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -104,7 +104,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
const __be32 *match_array = initial_match_array;
const __be32 *tmp, *imap, *imask, dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
- int imaplen, match, i;
+ int imaplen, match, i, rc = -EINVAL;
#ifdef DEBUG
of_print_phandle_args("of_irq_parse_raw: ", out_irq);
@@ -134,7 +134,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
pr_debug("of_irq_parse_raw: ipar=%s, size=%d\n", of_node_full_name(ipar), intsize);
if (out_irq->args_count != intsize)
- return -EINVAL;
+ goto fail;
/* Look for this #address-cells. We have to implement the old linux
* trick of looking for the parent here as some device-trees rely on it
@@ -153,8 +153,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
pr_debug(" -> addrsize=%d\n", addrsize);
/* Range check so that the temporary buffer doesn't overflow */
- if (WARN_ON(addrsize + intsize > MAX_PHANDLE_ARGS))
+ if (WARN_ON(addrsize + intsize > MAX_PHANDLE_ARGS)) {
+ rc = -EFAULT;
goto fail;
+ }
/* Precalculate the match array - this simplifies match loop */
for (i = 0; i < addrsize; i++)
@@ -240,10 +242,11 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
newintsize, newaddrsize);
/* Check for malformed properties */
- if (WARN_ON(newaddrsize + newintsize > MAX_PHANDLE_ARGS))
- goto fail;
- if (imaplen < (newaddrsize + newintsize))
+ if (WARN_ON(newaddrsize + newintsize > MAX_PHANDLE_ARGS)
+ || (imaplen < (newaddrsize + newintsize))) {
+ rc = -EFAULT;
goto fail;
+ }
imap += newaddrsize + newintsize;
imaplen -= newaddrsize + newintsize;
@@ -271,11 +274,13 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
ipar = newpar;
newpar = NULL;
}
+ rc = -ENOENT; /* No interrupt-map found */
+
fail:
of_node_put(ipar);
of_node_put(newpar);
- return -EINVAL;
+ return rc;
}
EXPORT_SYMBOL_GPL(of_irq_parse_raw);
diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 2306313..c175d9c 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -93,7 +93,15 @@ int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq
goto err;
return 0;
err:
- dev_err(&pdev->dev, "of_irq_parse_pci() failed with rc=%d\n", rc);
+ if (rc == -ENOENT) {
+ dev_warn(&pdev->dev,
+ "%s: no interrupt-map found, INTx interrupts not available\n",
+ __func__);
+ pr_warn_once("%s: possibly some PCI slots don't have level triggered interrupts capability\n",
+ __func__);
+ } else {
+ dev_err(&pdev->dev, "%s: failed with rc=%d\n", __func__, rc);
+ }
return rc;
}
EXPORT_SYMBOL_GPL(of_irq_parse_pci);
--
2.1.0
^ permalink raw reply related
* RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation
From: Punnaiah Choudary Kalluri @ 2016-12-05 13:56 UTC (permalink / raw)
To: Boris Brezillon, Marek Vasut
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, richard@nod.at,
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
linux-mtd@lists.infradead.org, kalluripunnaiahchoudary@gmail.com,
kpc528@gmail.com, Michal Simek, cyrille.pitchen@atmel.com,
computersforpeace@gmail.com, dwmw2@infradead.org
In-Reply-To: <20161205093630.650451d7@bbrezillon>
> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com]
> Sent: Monday, December 05, 2016 2:07 PM
> To: Marek Vasut <marek.vasut@gmail.com>
> Cc: Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> dwmw2@infradead.org; computersforpeace@gmail.com; richard@nod.at;
> cyrille.pitchen@atmel.com; robh+dt@kernel.org; mark.rutland@arm.com;
> linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> kalluripunnaiahchoudary@gmail.com; kpc528@gmail.com; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
>
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut <marek.vasut@gmail.com> wrote:
>
> > On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > > This patch adds the dts binding document for arasan nand flash
> > > controller.
> > >
> > > Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> > > Acked-by: Rob Herring <robh@kernel.org>
> > > ---
> > > changes in v6:
> > > - Removed num-cs property
> > > - Separated nandchip from nand controller changes in v5:
> > > - None
> > > Changes in v4:
> > > - Added num-cs property
> > > - Added clock support
> > > Changes in v3:
> > > - None
> > > Changes in v2:
> > > - None
> > > ---
> > > .../devicetree/bindings/mtd/arasan_nfc.txt | 38
> ++++++++++++++++++++++
> > > 1 file changed, 38 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > new file mode 100644
> > > index 0000000..dcbe7ad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > @@ -0,0 +1,38 @@
> > > +Arasan Nand Flash Controller with ONFI 3.1 support
> >
> > Arasan NAND Flash ...
> >
> > > +Required properties:
> > > +- compatible: Should be "arasan,nfc-v3p10"
> >
> > This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> > some fallback option which doesn't encode IP version in the compat
> > string ?
>
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific compatible
> strings is a good solution to avoid breaking the DT ABI.
Yes. I am also thinking the same. Arasan controller doesn't have the register
to indicate the IP version number.
>
> >
> > Also, shouldn't quirks be handled by DT props instead of effectively
> > encoding them into the compatible string ?
>
> Well, from my experience, it's better to hide as much as possible behind the
> compatible. This way, if new quirks are needed for a specific revision, you can
> update the driver without having to change the DT.
>
Agree.
Regards,
Punnaiah
> >
> > > +- reg: Memory map for module access
> > > +- interrupt-parent: Interrupt controller the interrupt is routed
> > > +through
> > > +- interrupts: Should contain the interrupt for the device
> > > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > > + (See clock bindings for details)
> > > +- clocks: Clock phandles (see clock bindings for details)
> > > +
> > > +Optional properties:
> > > +- arasan,has-mdma: Enables Dma support
> >
> > 'Enables DMA support' , with DMA in caps.
> >
> > > +for nand partition information please refer the below file
> >
> > For NAND ...
> >
> > > +Documentation/devicetree/bindings/mtd/partition.txt
> > > +
> > > +Example:
> > > + nand0: nand@ff100000 {
> > > + compatible = "arasan,nfc-v3p10"
> > > + reg = <0x0 0xff100000 0x1000>;
> > > + clock-name = "clk_sys", "clk_flash"
> > > + clocks = <&misc_clk &misc_clk>;
> > > + interrupt-parent = <&gic>;
> > > + interrupts = <0 14 4>;
> > > + arasan,has-mdma;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>
> > > +
> > > + nand@0 {
> > > + reg = <0>
> > > + partition@0 {
> > > + label = "filesystem";
> > > + reg = <0x0 0x0 0x1000000>;
> > > + };
> > > + (...)
> > > + };
> > > + };
> > >
> >
> >
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* RE: [PATCH v6 2/2] mtd: nand: Add support for Arasan Nand Flash Controller
From: Punnaiah Choudary Kalluri @ 2016-12-05 13:55 UTC (permalink / raw)
To: Marek Vasut, dwmw2@infradead.org, computersforpeace@gmail.com,
boris.brezillon@free-electrons.com, richard@nod.at,
cyrille.pitchen@atmel.com, robh+dt@kernel.org,
mark.rutland@arm.com
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Michal Simek, kalluripunnaiahchoudary@gmail.com, kpc528@gmail.com,
linux-mtd@lists.infradead.org
In-Reply-To: <7a8fe6c9-b53f-a16f-19f7-6ce4a83672a2@gmail.com>
Hi Marek,
Thanks for the review and comments.
> -----Original Message-----
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: Monday, December 05, 2016 10:10 AM
> To: Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> dwmw2@infradead.org; computersforpeace@gmail.com;
> boris.brezillon@free-electrons.com; richard@nod.at;
> cyrille.pitchen@atmel.com; robh+dt@kernel.org; mark.rutland@arm.com
> Cc: linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> kalluripunnaiahchoudary@gmail.com; kpc528@gmail.com; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [PATCH v6 2/2] mtd: nand: Add support for Arasan Nand Flash
> Controller
>
> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > Added the basic driver for Arasan Nand Flash Controller used in
> > Zynq UltraScale+ MPSoC. It supports only Hw Ecc and upto 24bit
> > correction.
>
> Ummm, NAND, ECC, can you fix the acronyms to be in caps ?
>
Ok
> > Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> > ---
> > Chnages in v6:
> > - Addressed most of the Brian and Boris comments
> > - Separated the nandchip from the nand controller
> > - Removed the ecc lookup table from driver
> > - Now use framework nand waitfunction and readoob
> > - Fixed the compiler warning
> > - Adapted the new frameowrk changes related to ecc and ooblayout
> > - Disabled the clocks after the nand_reelase
> > - Now using only one completion object
> > - Boris suggessions like adapting cmd_ctrl and rework on read/write byte
> > are not implemented and i will patch them later
> > - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr
> will
> > implement later once the basic driver is mainlined.
> > Changes in v5:
> > - Renamed the driver filei as arasan_nand.c
> > - Fixed all comments relaqted coding style
> > - Fixed comments related to propagating the errors
> > - Modified the anfc_write_page_hwecc as per the write_page
> > prototype
> > Changes in v4:
> > - Added support for onfi timing mode configuration
> > - Added clock supppport
> > - Added support for multiple chipselects
> > Changes in v3:
> > - Removed unused variables
> > - Avoided busy loop and used jifies based implementation
> > - Fixed compiler warnings "right shift count >= width of type"
> > - Removed unneeded codei and improved error reporting
> > - Added onfi version check to ensure reading the valid address cycles
> > Changes in v2:
> > - Added missing of.h to avoid kbuild system report erro
> > ---
> > drivers/mtd/nand/Kconfig | 8 +
> > drivers/mtd/nand/Makefile | 1 +
> > drivers/mtd/nand/arasan_nand.c | 974
> +++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 983 insertions(+)
> > create mode 100644 drivers/mtd/nand/arasan_nand.c
> >
> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > index 7b7a887..e831f4e 100644
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -569,4 +569,12 @@ config MTD_NAND_MTK
> > Enables support for NAND controller on MTK SoCs.
> > This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> >
> > +config MTD_NAND_ARASAN
> > + tristate "Support for Arasan Nand Flash controller"
> > + depends on HAS_IOMEM
> > + depends on HAS_DMA
> > + help
> > + Enables the driver for the Arasan Nand Flash controller on
> > + Zynq UltraScale+ MPSoC.
> > +
> > endif # MTD_NAND
> > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> > index cafde6f..44b8b00 100644
> > --- a/drivers/mtd/nand/Makefile
> > +++ b/drivers/mtd/nand/Makefile
> > @@ -58,5 +58,6 @@ obj-$(CONFIG_MTD_NAND_HISI504) +=
> hisi504_nand.o
> > obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/
> > obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o
> > obj-$(CONFIG_MTD_NAND_MTK) += mtk_nand.o mtk_ecc.o
> > +obj-$(CONFIG_MTD_NAND_ARASAN) += arasan_nand.o
>
> Keep the list at least reasonably sorted.
Ok
>
> > nand-objs := nand_base.o nand_bbt.o nand_timings.o
> > diff --git a/drivers/mtd/nand/arasan_nand.c
> b/drivers/mtd/nand/arasan_nand.c
> > new file mode 100644
> > index 0000000..6b0670e
> > --- /dev/null
> > +++ b/drivers/mtd/nand/arasan_nand.c
> > @@ -0,0 +1,974 @@
> > +/*
> > + * Arasan Nand Flash Controller Driver
>
> NAND
>
> > + * Copyright (C) 2014 - 2015 Xilinx, Inc.
>
> It's 2016 now ...
Sorry :). Yes, I will update.
>
> > + * 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; either version 2 of the License, or (at your
> > + * option) any later version.
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io-64-nonatomic-lo-hi.h>
> > +#include <linux/module.h>
> > +#include <linux/mtd/mtd.h>
> > +#include <linux/mtd/nand.h>
> > +#include <linux/mtd/partitions.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define DRIVER_NAME "arasan_nand"
> > +#define EVNT_TIMEOUT 1000
>
> Rename to EVENT_TIMEOUT_<units> to make this less cryptic
Ok.
>
> > +#define STATUS_TIMEOUT 2000
>
> DTTO
Thanks. Just realized that it is unused. I will remove this.
>
> > +#define PKT_OFST 0x00
> > +#define MEM_ADDR1_OFST 0x04
> > +#define MEM_ADDR2_OFST 0x08
> > +#define CMD_OFST 0x0C
> > +#define PROG_OFST 0x10
> > +#define INTR_STS_EN_OFST 0x14
> > +#define INTR_SIG_EN_OFST 0x18
> > +#define INTR_STS_OFST 0x1C
> > +#define READY_STS_OFST 0x20
> > +#define DMA_ADDR1_OFST 0x24
> > +#define FLASH_STS_OFST 0x28
> > +#define DATA_PORT_OFST 0x30
> > +#define ECC_OFST 0x34
> > +#define ECC_ERR_CNT_OFST 0x38
> > +#define ECC_SPR_CMD_OFST 0x3C
> > +#define ECC_ERR_CNT_1BIT_OFST 0x40
> > +#define ECC_ERR_CNT_2BIT_OFST 0x44
> > +#define DMA_ADDR0_OFST 0x50
> > +#define DATA_INTERFACE_REG 0x6C
>
> Why are some things suffixed with _OFST and some with _REG ? Consistency
> please. Using ARASAN_ prefix, ie. #define ARASAN_FOO 0xbar to define
> regs would be nice.
>
Ok. I will maintain the consistency. Since all these definitions for arasan controller
and I feel adding the prefix is no value and some places it will go beyond 80 col limit.
Different reviewers have different opinions here. I am following the above convention.
> > +#define PKT_CNT_SHIFT 12
> > +
> > +#define ECC_ENABLE BIT(31)
> > +#define DMA_EN_MASK GENMASK(27, 26)
> > +#define DMA_ENABLE 0x2
> > +#define DMA_EN_SHIFT 26
> > +#define REG_PAGE_SIZE_MASK GENMASK(25, 23)
> > +#define REG_PAGE_SIZE_SHIFT 23
> > +#define REG_PAGE_SIZE_512 0
> > +#define REG_PAGE_SIZE_1K 5
> > +#define REG_PAGE_SIZE_2K 1
> > +#define REG_PAGE_SIZE_4K 2
> > +#define REG_PAGE_SIZE_8K 3
> > +#define REG_PAGE_SIZE_16K 4
> > +#define CMD2_SHIFT 8
> > +#define ADDR_CYCLES_SHIFT 28
> > +
> > +#define XFER_COMPLETE BIT(2)
> > +#define READ_READY BIT(1)
> > +#define WRITE_READY BIT(0)
> > +#define MBIT_ERROR BIT(3)
> > +#define ERR_INTRPT BIT(4)
> > +
> > +#define PROG_PGRD BIT(0)
> > +#define PROG_ERASE BIT(2)
> > +#define PROG_STATUS BIT(3)
> > +#define PROG_PGPROG BIT(4)
> > +#define PROG_RDID BIT(6)
> > +#define PROG_RDPARAM BIT(7)
> > +#define PROG_RST BIT(8)
> > +#define PROG_GET_FEATURE BIT(9)
> > +#define PROG_SET_FEATURE BIT(10)
> > +
> > +#define ONFI_STATUS_FAIL BIT(0)
> > +#define ONFI_STATUS_READY BIT(6)
> > +
> > +#define PG_ADDR_SHIFT 16
> > +#define BCH_MODE_SHIFT 25
> > +#define BCH_EN_SHIFT 27
> > +#define ECC_SIZE_SHIFT 16
> > +
> > +#define MEM_ADDR_MASK GENMASK(7, 0)
> > +#define BCH_MODE_MASK GENMASK(27, 25)
> > +
> > +#define CS_MASK GENMASK(31, 30)
> > +#define CS_SHIFT 30
> > +
> > +#define PAGE_ERR_CNT_MASK GENMASK(16, 8)
> > +#define PKT_ERR_CNT_MASK GENMASK(7, 0)
> > +
> > +#define NVDDR_MODE BIT(9)
> > +#define NVDDR_TIMING_MODE_SHIFT 3
> > +
> > +#define ONFI_ID_LEN 8
> > +#define TEMP_BUF_SIZE 512
> > +#define NVDDR_MODE_PACKET_SIZE 8
> > +#define SDR_MODE_PACKET_SIZE 4
> > +
> > +#define ONFI_DATA_INTERFACE_NVDDR (1 << 4)
>
> BIT() ?
Sure.
>
>
> [...]
>
> > +struct anfc {
> > + struct nand_hw_control controller;
> > + struct list_head chips;
> > + struct device *dev;
> > + void __iomem *base;
> > + int curr_cmd;
> > + struct clk *clk_sys;
> > + struct clk *clk_flash;
> > + bool dma;
> > + bool err;
> > + bool iswriteoob;
> > + u8 buf[TEMP_BUF_SIZE];
> > + int irq;
> > + u32 bufshift;
> > + int csnum;
> > + struct completion evnt;
>
> event ?
Ok
>
> > +};
>
> [...]
>
> > +static void anfc_prepare_cmd(struct anfc *nfc, u8 cmd1, u8 cmd2, u8
> dmamode,
> > + u32 pagesize, u8 addrcycles)
> > +{
> > + u32 regval;
> > +
> > + regval = cmd1 | (cmd2 << CMD2_SHIFT);
> > + if (dmamode && nfc->dma)
> > + regval |= DMA_ENABLE << DMA_EN_SHIFT;
> > + if (addrcycles)
> > + regval |= addrcycles << ADDR_CYCLES_SHIFT;
> > + if (pagesize)
> > + regval |= anfc_page(pagesize) << REG_PAGE_SIZE_SHIFT;
>
> Drop the if (foo), if it's zero, the regval would be OR'd with zero.
Ok.
>
> > + writel(regval, nfc->base + CMD_OFST);
> > +}
> > +
> > +static int anfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> > + int page)
> > +{
> > + struct anfc *nfc = to_anfc(chip->controller);
> > +
> > + nfc->iswriteoob = true;
> > + chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page);
> > + chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> > + nfc->iswriteoob = false;
> > +
> > + return 0;
> > +}
> > +
> > +static void anfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> > +{
> > + u32 pktcount, pktsize, eccintr = 0;
> > + unsigned int buf_rd_cnt = 0;
> > + u32 *bufptr = (u32 *)buf;
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + struct anfc *nfc = to_anfc(chip->controller);
> > + dma_addr_t paddr;
> > +
> > + if (nfc->curr_cmd == NAND_CMD_READ0) {
> > + pktsize = achip->pktsize;
> > + pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
> > + if (!achip->bch)
> > + eccintr = MBIT_ERROR;
> > + } else {
> > + pktsize = len;
> > + pktcount = 1;
> > + }
> > +
> > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > + if (nfc->dma) {
> > + paddr = dma_map_single(nfc->dev, buf, len,
> DMA_FROM_DEVICE);
> > + if (dma_mapping_error(nfc->dev, paddr)) {
> > + dev_err(nfc->dev, "Read buffer mapping error");
> > + return;
> > + }
> > + lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> > + anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
> > + writel(PROG_PGRD, nfc->base + PROG_OFST);
> > + anfc_wait_for_event(nfc);
> > + dma_unmap_single(nfc->dev, paddr, len,
> DMA_FROM_DEVICE);
>
> Split this function into anfc_read_buf() and then anfc_read_buf_pio()
> and anfc_read_buf_dma() to avoid this ugliness. Also, does this handle
> any errors in any way ? Looks like it ignores all errors, so please fix.
>
Ok. Regarding errors, it handles the errors except checking for contiguous
Address region for the given address. I will add this in next version.
> > + return;
> > + }
> > +
> > + anfc_enable_intrs(nfc, (READ_READY | eccintr));
> > + writel(PROG_PGRD, nfc->base + PROG_OFST);
> > +
> > + while (buf_rd_cnt < pktcount) {
> > + anfc_wait_for_event(nfc);
> > + buf_rd_cnt++;
> > +
> > + if (buf_rd_cnt == pktcount)
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +
> > + readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> > + bufptr += (pktsize / 4);
> > +
> > + if (buf_rd_cnt < pktcount)
> > + anfc_enable_intrs(nfc, (READ_READY | eccintr));
> > + }
> > +
> > + anfc_wait_for_event(nfc);
> > +}
> > +
> > +static void anfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int
> len)
> > +{
> > + u32 pktcount, pktsize;
> > + unsigned int buf_wr_cnt = 0;
> > + u32 *bufptr = (u32 *)buf;
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + struct anfc *nfc = to_anfc(chip->controller);
> > + dma_addr_t paddr;
> > +
> > + if (nfc->iswriteoob) {
> > + pktsize = len;
> > + pktcount = 1;
> > + } else {
> > + pktsize = achip->pktsize;
> > + pktcount = mtd->writesize / pktsize;
> > + }
>
> This looks like a copy of the read path. Can these two functions be
> parametrized and merged ?
>
Ok.
> > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > + if (nfc->dma) {
> > + paddr = dma_map_single(nfc->dev, (void *)buf, len,
> > + DMA_TO_DEVICE);
> > + if (dma_mapping_error(nfc->dev, paddr)) {
> > + dev_err(nfc->dev, "Write buffer mapping error");
> > + return;
> > + }
> > + lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > + writel(PROG_PGPROG, nfc->base + PROG_OFST);
> > + anfc_wait_for_event(nfc);
> > + dma_unmap_single(nfc->dev, paddr, len, DMA_TO_DEVICE);
> > + return;
> > + }
> > +
> > + anfc_enable_intrs(nfc, WRITE_READY);
> > + writel(PROG_PGPROG, nfc->base + PROG_OFST);
> > +
> > + while (buf_wr_cnt < pktcount) {
> > + anfc_wait_for_event(nfc);
> > + buf_wr_cnt++;
> > + if (buf_wr_cnt == pktcount)
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +
> > + writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> > + bufptr += (pktsize / 4);
> > +
> > + if (buf_wr_cnt < pktcount)
> > + anfc_enable_intrs(nfc, WRITE_READY);
> > + }
> > +
> > + anfc_wait_for_event(nfc);
> > +}
>
>
> [...]
>
> > +static void anfc_writefifo(struct anfc *nfc, u32 prog, u32 size, u8 *buf)
> > +{
> > + u32 *bufptr = (u32 *)buf;
> > +
> > + anfc_enable_intrs(nfc, WRITE_READY);
> > +
> > + writel(prog, nfc->base + PROG_OFST);
> > + anfc_wait_for_event(nfc);
> > +
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > + writesl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
>
> use ioread32_rep and iowrite32_rep , otherwise this won't compile on x86
> with COMPILE_TEST.
>
Ok. I have just got the kbuild notification for x86 system. I will fix this.
> > + anfc_wait_for_event(nfc);
> > +}
> > +
> > +static void anfc_readfifo(struct anfc *nfc, u32 prog, u32 size)
> > +{
> > + u32 *bufptr = (u32 *)nfc->buf;
> > +
> > + anfc_enable_intrs(nfc, READ_READY);
> > +
> > + writel(prog, nfc->base + PROG_OFST);
> > + anfc_wait_for_event(nfc);
> > +
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > + readsl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
>
> See above
>
> > + anfc_wait_for_event(nfc);
> > +}
>
>
> [...]
>
> > +static void anfc_select_chip(struct mtd_info *mtd, int num)
> > +{
> > + u32 val;
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + struct anfc *nfc = to_anfc(chip->controller);
> > +
> > + if (num == -1)
> > + return;
> > +
> > + val = readl(nfc->base + MEM_ADDR2_OFST);
> > + val = (val & ~(CS_MASK)) | (achip->csnum << CS_SHIFT);
> > + val = (val & ~(BCH_MODE_MASK)) | (achip->bchmode <<
> BCH_MODE_SHIFT);
>
> Just rewrite this as a series of val &= ~(foo | bar); val |= baz | quux;
> for clarity sake.
>
Ok.
> > + writel(val, nfc->base + MEM_ADDR2_OFST);
> > + nfc->csnum = achip->csnum;
> > + writel(achip->eccval, nfc->base + ECC_OFST);
> > + writel(achip->inftimeval, nfc->base + DATA_INTERFACE_REG);
> > +}
> > +
> > +static irqreturn_t anfc_irq_handler(int irq, void *ptr)
> > +{
> > + struct anfc *nfc = ptr;
> > + u32 regval = 0, status;
> > +
> > + status = readl(nfc->base + INTR_STS_OFST);
> > + if (status & XFER_COMPLETE) {
> > + complete(&nfc->evnt);
> > + regval |= XFER_COMPLETE;
>
> Can the complete() be invoked multiple times ? That seems a bit odd.
>
I will check and fix this.
> > + }
> > +
> > + if (status & READ_READY) {
> > + complete(&nfc->evnt);
> > + regval |= READ_READY;
> > + }
> > +
> > + if (status & WRITE_READY) {
> > + complete(&nfc->evnt);
> > + regval |= WRITE_READY;
> > + }
> > +
> > + if (status & MBIT_ERROR) {
> > + nfc->err = true;
> > + complete(&nfc->evnt);
> > + regval |= MBIT_ERROR;
> > + }
> > +
> > + if (regval) {
> > + writel(regval, nfc->base + INTR_STS_OFST);
> > + writel(0, nfc->base + INTR_STS_EN_OFST);
> > + writel(0, nfc->base + INTR_SIG_EN_OFST);
> > +
> > + return IRQ_HANDLED;
> > + }
> > +
> > + return IRQ_NONE;
> > +}
> > +
> > +static int anfc_onfi_set_features(struct mtd_info *mtd, struct nand_chip
> *chip,
> > + int addr, uint8_t *subfeature_param)
> > +{
> > + struct anfc *nfc = to_anfc(chip->controller);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + int status;
> > +
> > + if (!chip->onfi_version || !(le16_to_cpu(chip-
> >onfi_params.opt_cmd)
> > + & ONFI_OPT_CMD_SET_GET_FEATURES))
>
> Split this into two conditions to improve readability.
>
Ok.
> > + return -EINVAL;
> > +
> > + chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
> > + anfc_writefifo(nfc, PROG_SET_FEATURE, achip->spktsize,
> > + subfeature_param);
> > +
> > + status = chip->waitfunc(mtd, chip);
> > + if (status & NAND_STATUS_FAIL)
> > + return -EIO;
> > +
> > + return 0;
> > +}
> > +
> > +static int anfc_init_timing_mode(struct anfc *nfc,
> > + struct anfc_nand_chip *achip)
> > +{
> > + int mode, err;
> > + unsigned int feature[2];
> > + u32 inftimeval;
> > + struct nand_chip *chip = &achip->chip;
> > + struct mtd_info *mtd = nand_to_mtd(chip);
> > +
> > + memset(feature, 0, NVDDR_MODE_PACKET_SIZE);
> > + /* Get nvddr timing modes */
> > + mode = onfi_get_sync_timing_mode(chip) & 0xff;
> > + if (!mode) {
> > + mode = fls(onfi_get_async_timing_mode(chip)) - 1;
> > + inftimeval = mode;
> > + } else {
> > + mode = fls(mode) - 1;
> > + inftimeval = NVDDR_MODE | (mode <<
> NVDDR_TIMING_MODE_SHIFT);
> > + mode |= ONFI_DATA_INTERFACE_NVDDR;
> > + }
> > +
> > + feature[0] = mode;
> > + chip->select_chip(mtd, achip->csnum);
> > + err = chip->onfi_set_features(mtd, chip,
> ONFI_FEATURE_ADDR_TIMING_MODE,
> > + (uint8_t *)feature);
> > + chip->select_chip(mtd, -1);
> > + if (err)
> > + return err;
> > +
> > + achip->inftimeval = inftimeval;
> > +
> > + if (mode & ONFI_DATA_INTERFACE_NVDDR)
> > + achip->spktsize = NVDDR_MODE_PACKET_SIZE;
> > +
> > + return 0;
> > +}
>
> [...]
>
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Xilinx, Inc");
>
> There should be a contact with email address here.
>
I think there is an alias for Xilinx, Inc in maintainers list.
If not I will update it.
Regards,
Punnaiah
> > +MODULE_DESCRIPTION("Arasan NAND Flash Controller Driver");
> >
>
>
> --
> Best regards,
> Marek Vasut
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
From: Mark Brown @ 2016-12-05 13:37 UTC (permalink / raw)
To: Romain Perier
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, 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: <20161201102719.4291-3-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]
On Thu, Dec 01, 2016 at 11:27:16AM +0100, Romain Perier wrote:
> Changes in v3:
> - Don't enable the fifo mode based on the compatible string, we introduce
> a module parameter "pio_mode". By default this option is set to zero, so
> the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
> mode.
Why? If the hardware supports a more efficient mode of operation it
doesn't seem sensible not to use it.
> - int i;
> + int i, ret = 0;
Coding style - don't combine initialized and non-initalized variables on
one line.
> 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 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
> return ret;
> }
This appears to be a random unrelated change, should probably be part of
the initial driver commit.
> +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 */
So this only supports single duplex transfers but there doesn't seem to
be anything that enforces this.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2 4/4] ARM: dts: hix5hd2: add gmac generic compatible and clock names
From: Dongpo Li @ 2016-12-05 13:28 UTC (permalink / raw)
To: robh+dt, mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
yisen.zhuang, salil.mehta, davem, arnd, andrew
Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
linux-kernel, Dongpo Li
In-Reply-To: <1480944481-118803-1-git-send-email-lidongpo@hisilicon.com>
Add gmac generic compatible and clock names.
Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
arch/arm/boot/dts/hisi-x5hd2.dtsi | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/hisi-x5hd2.dtsi b/arch/arm/boot/dts/hisi-x5hd2.dtsi
index fdcc23d..0da76c5 100644
--- a/arch/arm/boot/dts/hisi-x5hd2.dtsi
+++ b/arch/arm/boot/dts/hisi-x5hd2.dtsi
@@ -436,18 +436,20 @@
};
gmac0: ethernet@1840000 {
- compatible = "hisilicon,hix5hd2-gmac";
+ compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
reg = <0x1840000 0x1000>,<0x184300c 0x4>;
interrupts = <0 71 4>;
clocks = <&clock HIX5HD2_MAC0_CLK>;
+ clock-names = "mac_core";
status = "disabled";
};
gmac1: ethernet@1841000 {
- compatible = "hisilicon,hix5hd2-gmac";
+ compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
reg = <0x1841000 0x1000>,<0x1843010 0x4>;
interrupts = <0 72 4>;
clocks = <&clock HIX5HD2_MAC1_CLK>;
+ clock-names = "mac_core";
status = "disabled";
};
--
2.8.2
^ permalink raw reply related
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