* [PATCH] arm64: dts: allwinner: pine64: Add remaining UART aliases
From: Andreas Färber @ 2017-04-14 19:08 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Andreas Färber, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
Mark Rutland, Catalin Marinas, Will Deacon,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Enabling uart2 node currently leads to a /dev/ttyS1 device, with ttyS0..4
always present, causing confusion on the user's part.
dtc cannot resolve an overlay's &uart2 reference for strings, only for
phandles, so it would need to hardcode the full node path.
Avoid this and enforce reliable numbering by adding serialX aliases for:
UART1 - on Wifi/BT connector
UART2 - on Pi-2 connector
UART3 - on Euler connector
UART4 - on Euler connector
Signed-off-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
---
arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index c680ed385da3..db6c0f36999e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -52,6 +52,10 @@
aliases {
serial0 = &uart0;
+ serial1 = &uart1;
+ serial2 = &uart2;
+ serial3 = &uart3;
+ serial4 = &uart4;
};
chosen {
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* WHAT HAS KEPT YOU?
From: Mr Light Adam @ 2017-04-14 18:39 UTC (permalink / raw)
Hello Dear,
What Has Kept You Waiting To Claim Your $650,000.00 USD Compensation Award?
This said fund was issued out by the UNITED NATIONS To compensate
you.Please If You Have Not Claim Yours,Kindly Write Back To Me
Immediately You Receive This Information For An Urgent Confirmation
And Release Of Your Fund To You Without Delays as your email was
listed among to be compensated.
Best Regards,
Mr Light Adam
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 4/9] drm/stm: Add STM32 LTDC driver
From: Eric Anholt @ 2017-04-14 18:38 UTC (permalink / raw)
To: Yannick Fertre, Alexandre TORGUE, Thierry Reding, David Airlie,
Maxime Coquelin, Russell King, Mark Rutland, Rob Herring,
Arnd Bergmann, Benjamin Gaignard
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-F5mvAk5X5gdBDgjK7y7TUQ,
Philippe Cornu, Fabien Dessenne,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Mickael Reulier,
Vincent Abriou, Gabriel FERNANDEZ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1492164819-10513-5-git-send-email-yannick.fertre-qxv4g6HH51o@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]
Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org> writes:
> This controller provides output signals to interface directly a variety
> of LCD and TFT panels. These output signals are: RGB signals
> (up to 24bpp), vertical & horizontal synchronisations, data enable and
> the pixel clock.
>
> Reviewed-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> Signed-off-by: Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>
I squashed in one tiny fix for the new compiler warning due to *event
being unused in disable() now, applied Neil's review and my own, and
pushed patches 1-4. Patches 5-9 will have to go through your arm-soc
trees for the platform.
Congrats!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] net: macb: Add hardware PTP support
From: Richard Cochran @ 2017-04-14 18:28 UTC (permalink / raw)
To: Rafal Ozieblo
Cc: David Miller, nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
harinikatakamlinux-Re5JQEeQqe8AvxtiuMwx3w,
harini.katakam-gjFFaj9aHVfQT0dZR+AlfA,
Andrei.Pistirica-UWL1GkI3JZL3oGB3hsPCZA
In-Reply-To: <1492090763-15686-1-git-send-email-rafalo-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>
On Thu, Apr 13, 2017 at 02:39:23PM +0100, Rafal Ozieblo wrote:
> This patch is based on original Harini's patch and Andrei's patch,
> implemented in a separate file to ease the review/maintanance
> and integration with other platforms.
Please see if you can break this patch into 2 parts:
1. SO_TIMESTAMPING
2. PHC support
> This driver does support GEM-GXL:
"This driver supports GEM-GXL:"
> - HW time stamp on the PTP Ethernet packets are received using the
> SO_TIMESTAMPING API. Where timers are obtained from the dma buffer
> descriptors
This text is poor. No "timers" are obtained but rather time stamps.
Also, second sentence is not a sentence. (An english sentence has a
noun and a verb.)
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 59d459b..603bac1 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -826,6 +826,15 @@ static void macb_tx_interrupt(struct macb_queue *queue)
>
> /* First, update TX stats if needed */
> if (skb) {
> +#ifdef CONFIG_MACB_USE_HWSTAMP
No need for ifdef here. Instead, let gem_ptp_do_txstamp() return -1.
> + if (gem_ptp_do_txstamp(queue, skb, desc) == 0) {
> + /* skb now belongs to timestamp buffer
> + * and will be removed later
> + */
> + tx_skb->skb = NULL;
> + schedule_work(&queue->tx_ts_task);
> + }
> +#endif
> netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
> macb_tx_ring_wrap(bp, tail),
> skb->data);
> @@ -992,6 +1001,10 @@ static int gem_rx(struct macb *bp, int budget)
> bp->dev->stats.rx_packets++;
> bp->dev->stats.rx_bytes += skb->len;
>
> +#ifdef CONFIG_MACB_USE_HWSTAMP
No ifdef needed.
> + gem_ptp_do_rxstamp(bp, skb, desc);
> +#endif
> +
> #if defined(DEBUG) && defined(VERBOSE_DEBUG)
> netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
> skb->len, skb->csum);
> @@ -1314,6 +1327,11 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
> queue_writel(queue, ISR, MACB_BIT(HRESP));
> }
>
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> + if (status & MACB_PTP_INT_MASK)
Can't you use IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) here?
> + macb_ptp_int(queue, status);
> +#endif
> +
> status = queue_readl(queue, ISR);
> }
>
> @@ -1643,8 +1661,10 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
> /* Make newly initialized descriptor visible to hardware */
> wmb();
> -
> - skb_tx_timestamp(skb);
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> + if (!bp->ptp_hw_support)
> +#endif
> + skb_tx_timestamp(skb);
This is wrong. You should call skb_tx_timestamp() unconditionally,
but be sure to set SKBTX_IN_PROGRESS when appropriate.
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 2606970..5295045 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -11,6 +11,9 @@
> #define _MACB_H
>
> #include <linux/phy.h>
> +#include <linux/ptp_clock.h>
You don't need to include ptp_clock.h.
> +#include <linux/ptp_clock_kernel.h>
> +#include <linux/net_tstamp.h>
>
> #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
> #define MACB_EXT_DESC
...
> @@ -527,6 +595,8 @@
> #define queue_readl(queue, reg) (queue)->bp->macb_reg_readl((queue)->bp, (queue)->reg)
> #define queue_writel(queue, reg, value) (queue)->bp->macb_reg_writel((queue)->bp, (queue)->reg, (value))
>
> +#define PTP_TS_BUFFER_SIZE 128 /* must be power of 2 */
> +
> /* Conditional GEM/MACB macros. These perform the operation to the correct
> * register dependent on whether the device is a GEM or a MACB. For registers
> * and bitfields that are common across both devices, use macb_{read,write}l
> @@ -889,6 +959,18 @@ struct macb_config {
> int jumbo_max_len;
> };
>
> +#ifdef CONFIG_MACB_USE_HWSTAMP
No need for ifdef here.
> +struct tsu_incr {
> + u32 sub_ns;
> + u32 ns;
> +};
> +
> +struct gem_tx_ts {
> + struct sk_buff *skb;
> + struct macb_dma_desc_ptp desc_ptp;
> +};
> +#endif
> +
> struct macb_queue {
> struct macb *bp;
> int irq;
...
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
> new file mode 100755
> index 0000000..72a79c4
> --- /dev/null
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -0,0 +1,724 @@
> +/**
> + * 1588 PTP support for Cadence GEM device.
> + *
> + * Copyright (C) 2017 Cadence Design Systems - http://www.cadence.com
> + *
> + * Authors: Rafal Ozieblo <rafalo-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>
> + * Bartosz Folta <bfolta-vna1KIf7WgpBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 of
> + * the License as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/etherdevice.h>
> +#include <linux/platform_device.h>
> +#include <linux/time64.h>
> +#include <linux/ptp_classify.h>
> +#include <linux/if_ether.h>
> +#include <linux/if_vlan.h>
> +#include <linux/net_tstamp.h>
> +#include <linux/circ_buf.h>
> +#include <linux/spinlock.h>
> +
> +#include "macb.h"
> +
> +#define GEM_PTP_TIMER_NAME "gem-ptp-timer"
> +
> +static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp,
> + struct macb_dma_desc *desc)
> +{
> + if (bp->hw_dma_cap == HW_DMA_CAP_PTP)
> + return (struct macb_dma_desc_ptp *)
> + ((u8 *)desc + sizeof(struct macb_dma_desc));
> + if (bp->hw_dma_cap == HW_DMA_CAP_64B_PTP)
> + return (struct macb_dma_desc_ptp *)
> + ((u8 *)desc + sizeof(struct macb_dma_desc)
> + + sizeof(struct macb_dma_desc_64));
> + return NULL;
> +}
> +
> +static int gem_tsu_get_time(struct macb *bp, struct timespec64 *ts)
> +{
> + long first, second;
> + u32 secl, sech;
> + unsigned long flags;
> +
> + if (!bp || !ts)
> + return -EINVAL;
Useless test.
> +
> + spin_lock_irqsave(&bp->tsu_clk_lock, flags);
> + first = gem_readl(bp, TN);
> + secl = gem_readl(bp, TSL);
> + sech = gem_readl(bp, TSH);
> + second = gem_readl(bp, TN);
> +
> + /* test for nsec rollover */
> + if (first > second) {
> + /* if so, use later read & re-read seconds
> + * (assume all done within 1s)
> + */
> + ts->tv_nsec = gem_readl(bp, TN);
> + secl = gem_readl(bp, TSL);
> + sech = gem_readl(bp, TSH);
> + } else
> + ts->tv_nsec = first;
CodingStyle.
Also, this assignment does not need the lock...
> +
> + ts->tv_sec = (((u64)sech << GEM_TSL_SIZE) | secl)
> + & TSU_SEC_MAX_VAL;
... nor this one.
> +
> + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags);
> + return 0;
> +}
> +
> +static int gem_tsu_set_time(struct macb *bp, const struct timespec64 *ts)
> +{
> + u32 ns, sech, secl;
> + unsigned long flags;
> +
> + if (!bp || !ts)
> + return -EINVAL;
Useless test.
> +
> + secl = (u32)ts->tv_sec;
> + sech = (ts->tv_sec >> GEM_TSL_SIZE) & ((1 << GEM_TSH_SIZE) - 1);
> + ns = ts->tv_nsec;
> +
> + spin_lock_irqsave(&bp->tsu_clk_lock, flags);
> +
> + /* TSH doesn't latch the time and no atomicity! */
> + gem_writel(bp, TN, 0); /* clear to avoid overflow */
> + gem_writel(bp, TSH, sech);
> + /* write lower bits 2nd, for synchronized secs update */
> + gem_writel(bp, TSL, secl);
> + gem_writel(bp, TN, ns);
> +
> + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags);
> +
> + return 0;
> +}
> +
> +static int gem_tsu_incr_set(struct macb *bp, struct tsu_incr *incr_spec)
> +{
> + unsigned long flags;
> +
> + if (!bp || !incr_spec)
> + return -EINVAL;
Useless test.
> +
> + /* tsu_timer_incr register must be written after
> + * the tsu_timer_incr_sub_ns register and the write operation
> + * will cause the value written to the tsu_timer_incr_sub_ns register
> + * to take effect.
> + */
> + spin_lock_irqsave(&bp->tsu_clk_lock, flags);
> + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, incr_spec->sub_ns));
> + gem_writel(bp, TI, GEM_BF(NSINCR, incr_spec->ns));
> + spin_unlock_irqrestore(&bp->tsu_clk_lock, flags);
> +
> + return 0;
> +}
> +
> +static int gem_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> + struct tsu_incr incr_spec;
> + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
> + u64 adj;
> + u32 word;
> + bool neg_adj = false;
> +
> + if (!ptp)
> + return -EINVAL;
Useless test (or can ptp be null?)
> +
> + if (scaled_ppm < 0) {
> + neg_adj = true;
> + scaled_ppm = -scaled_ppm;
> + }
> +
> + /* Adjustment is relative to base frequency */
> + incr_spec.sub_ns = bp->tsu_incr.sub_ns;
> + incr_spec.ns = bp->tsu_incr.ns;
> +
> + /* scaling: unused(8bit) | ns(8bit) | fractions(16bit) */
> + word = ((u64)incr_spec.ns << GEM_SUBNSINCR_SIZE) + incr_spec.sub_ns;
> + adj = (u64)scaled_ppm * word;
> + /* Divide with rounding, equivalent to floating dividing:
> + * (temp / USEC_PER_SEC) + 0.5
> + */
> + adj += (USEC_PER_SEC >> 1);
> + adj >>= GEM_SUBNSINCR_SIZE; /* remove fractions */
> + adj = div_u64(adj, USEC_PER_SEC);
> + adj = neg_adj ? (word - adj) : (word + adj);
> +
> + incr_spec.ns = (adj >> GEM_SUBNSINCR_SIZE)
> + & ((1 << GEM_NSINCR_SIZE) - 1);
> + incr_spec.sub_ns = adj & ((1 << GEM_SUBNSINCR_SIZE) - 1);
> + gem_tsu_incr_set(bp, &incr_spec);
> + return 0;
> +}
> +
> +static int gem_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> + struct timespec64 now, then = ns_to_timespec64(delta);
> + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
> + u32 adj, sign = 0;
> +
> + if (!ptp)
> + return -EINVAL;
Useless test.
> +
> + if (delta < 0) {
> + sign = 1;
> + delta = -delta;
> + }
> +
> + if (delta > TSU_NSEC_MAX_VAL) {
> + gem_tsu_get_time(bp, &now);
> + if (sign)
> + now = timespec64_sub(now, then);
> + else
> + now = timespec64_add(now, then);
> +
> + gem_tsu_set_time(bp, (const struct timespec64 *)&now);
> + } else {
> + adj = (sign << GEM_ADDSUB_OFFSET) | delta;
> +
> + gem_writel(bp, TA, adj);
> + }
> +
> + return 0;
> +}
> +
> +static int gem_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
> +{
> + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
> +
> + if (!ptp || !ts)
> + return -EINVAL;
Useles test.
What is the point of this wrapper function anyhow? Please remove it.
> +
> + gem_tsu_get_time(bp, ts);
> + return 0;
> +}
> +
> +static int gem_ptp_settime(struct ptp_clock_info *ptp,
> + const struct timespec64 *ts)
> +{
> + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
> +
> + if (!ptp || !ts)
> + return -EINVAL;
Another useless function.
> + gem_tsu_set_time(bp, ts);
> + return 0;
> +}
> +
> +static int gem_ptp_enable(struct ptp_clock_info *ptp,
> + struct ptp_clock_request *rq, int on)
> +{
> + struct macb *bp = container_of(ptp, struct macb, ptp_clock_info);
> +
> + if (!ptp || !rq)
> + return -EINVAL;
Sigh.
> +
> + switch (rq->type) {
> + case PTP_CLK_REQ_EXTTS: /* Toggle TSU match interrupt */
> + if (on)
> + macb_writel(bp, IER, MACB_BIT(TCI));
No locking to protect IER and IDE?
> + else
> + macb_writel(bp, IDR, MACB_BIT(TCI));
> + break;
> + case PTP_CLK_REQ_PEROUT: /* Toggle Periodic output */
> + return -EOPNOTSUPP;
> + /* break; */
> + case PTP_CLK_REQ_PPS: /* Toggle TSU periodic (second) interrupt */
> + if (on)
> + macb_writel(bp, IER, MACB_BIT(SRI));
> + else
> + macb_writel(bp, IDR, MACB_BIT(SRI));
> + break;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> +static struct ptp_clock_info gem_ptp_caps_template = {
> + .owner = THIS_MODULE,
> + .name = GEM_PTP_TIMER_NAME,
> + .max_adj = 0,
> + .n_alarm = 1,
You can't support alarms, because they are not implemented in the PHC
subsystem at all.
> + .n_ext_ts = 1,
(see last 2 functions in this patch)
> + .n_per_out = 0,
> + .n_pins = 0,
> + .pps = 1,
> + .adjfine = gem_ptp_adjfine,
> + .adjtime = gem_ptp_adjtime,
> + .gettime64 = gem_ptp_gettime,
> + .settime64 = gem_ptp_settime,
> + .enable = gem_ptp_enable,
> +};
> +
> +static void gem_ptp_init_timer(struct macb *bp)
> +{
> + u32 rem = 0;
> +
> + bp->tsu_incr.ns = div_u64_rem(NSEC_PER_SEC, bp->tsu_rate, &rem);
> + if (rem) {
> + u64 adj = rem;
> +
> + adj <<= GEM_SUBNSINCR_SIZE;
> + bp->tsu_incr.sub_ns = div_u64(adj, bp->tsu_rate);
> + } else {
> + bp->tsu_incr.sub_ns = 0;
> + }
> +}
> +
> +static void gem_ptp_init_tsu(struct macb *bp)
> +{
> + struct timespec64 ts;
> +
> + /* 1. get current system time */
> + ts = ns_to_timespec64(ktime_to_ns(ktime_get_real()));
> +
> + /* 2. set ptp timer */
> + gem_tsu_set_time(bp, &ts);
> +
> + /* 3. set PTP timer increment value to BASE_INCREMENT */
> + gem_tsu_incr_set(bp, &bp->tsu_incr);
> +
> + gem_writel(bp, TA, 0);
> +}
> +
> +static void gem_ptp_clear_timer(struct macb *bp)
> +{
> + bp->tsu_incr.ns = 0;
> + bp->tsu_incr.sub_ns = 0;
> +
> + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, 0));
> + gem_writel(bp, TI, GEM_BF(NSINCR, 0));
> + gem_writel(bp, TA, 0);
> +}
> +
> +static int gem_hw_timestamp(struct macb *bp,
> + u32 dma_desc_ts_1, u32 dma_desc_ts_2, struct timespec64 *ts)
> +{
> + struct timespec64 tsu;
> +
> + ts->tv_sec = (GEM_BFEXT(DMA_SECH, dma_desc_ts_2) << GEM_DMA_SECL_SIZE) |
> + GEM_BFEXT(DMA_SECL, dma_desc_ts_1);
> + ts->tv_nsec = GEM_BFEXT(DMA_NSEC, dma_desc_ts_1);
> +
> + /* TSU overlaping workaround
> + * The timestamp only contains lower few bits of seconds,
> + * so add value from 1588 timer
> + */
> + gem_tsu_get_time(bp, &tsu);
> +
> + /* If the top bit is set in the timestamp,
> + * but not in 1588 timer, it has rolled over,
> + * so subtract max size
> + */
> + if ((ts->tv_sec & (GEM_DMA_SEC_TOP >> 1)) &&
> + !(tsu.tv_sec & (GEM_DMA_SEC_TOP >> 1)))
> + ts->tv_sec -= GEM_DMA_SEC_TOP;
> +
> + ts->tv_sec += ((~GEM_DMA_SEC_MASK) & (tsu.tv_sec));
> +
> + return 0;
> +}
> +
> +void gem_ptp_rxstamp(struct macb *bp, struct sk_buff *skb,
> + struct macb_dma_desc *desc)
> +{
> + struct timespec64 ts;
> + struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
> + struct macb_dma_desc_ptp *desc_ptp;
> +
> + if (GEM_BFEXT(DMA_RXVALID, desc->addr)) {
> + desc_ptp = macb_ptp_desc(bp, desc);
> + gem_hw_timestamp(bp, desc_ptp->ts_1, desc_ptp->ts_2, &ts);
> + memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
> + shhwtstamps->hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec);
> + }
> +}
> +
> +static void gem_tstamp_tx(struct macb *bp, struct sk_buff *skb,
> + struct macb_dma_desc_ptp *desc_ptp)
> +{
> + struct skb_shared_hwtstamps shhwtstamps;
> + struct timespec64 ts;
> +
> + gem_hw_timestamp(bp, desc_ptp->ts_1, desc_ptp->ts_2, &ts);
> + memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> + shhwtstamps.hwtstamp = ktime_set(ts.tv_sec, ts.tv_nsec);
> + skb_tstamp_tx(skb, &shhwtstamps);
> +}
> +
> +int gem_ptp_txstamp(struct macb_queue *queue, struct sk_buff *skb,
> + struct macb_dma_desc *desc)
> +{
> + struct gem_tx_ts *tx_timestamp;
> + struct macb_dma_desc_ptp *desc_ptp;
> + unsigned long head = queue->tx_ts_head;
> + unsigned long tail = READ_ONCE(queue->tx_ts_tail);
> +
> + if (!GEM_BFEXT(DMA_TXVALID, desc->ctrl))
> + return -EINVAL;
> +
> + if (CIRC_SPACE(head, tail, PTP_TS_BUFFER_SIZE) == 0)
> + return -ENOMEM;
> +
> + desc_ptp = macb_ptp_desc(queue->bp, desc);
> + tx_timestamp = &queue->tx_timestamps[head];
> + tx_timestamp->skb = skb;
> + tx_timestamp->desc_ptp.ts_1 = desc_ptp->ts_1;
> + tx_timestamp->desc_ptp.ts_2 = desc_ptp->ts_2;
> + /* move head */
> + smp_store_release(&queue->tx_ts_head,
> + (head + 1) & (PTP_TS_BUFFER_SIZE - 1));
> + return 0;
> +}
> +
> +static void gem_tx_timestamp_flush(struct work_struct *work)
> +{
> + struct macb_queue *queue =
> + container_of(work, struct macb_queue, tx_ts_task);
> + struct gem_tx_ts *tx_ts;
> + unsigned long head, tail;
> +
> + /* take current head */
> + head = smp_load_acquire(&queue->tx_ts_head);
> + tail = queue->tx_ts_tail;
> +
> + while (CIRC_CNT(head, tail, PTP_TS_BUFFER_SIZE)) {
> + tx_ts = &queue->tx_timestamps[tail];
> + gem_tstamp_tx(queue->bp, tx_ts->skb, &tx_ts->desc_ptp);
> + /* cleanup */
> + dev_kfree_skb_any(tx_ts->skb);
> + /* remove old tail */
> + smp_store_release(&queue->tx_ts_tail,
> + (tail + 1) & (PTP_TS_BUFFER_SIZE - 1));
> + tail = queue->tx_ts_tail;
> + }
> +}
> +
> +void gem_ptp_init(struct net_device *dev)
> +{
> + struct macb *bp = netdev_priv(dev);
> + unsigned int q;
> + struct macb_queue *queue;
> +
> + bp->ptp_clock_info = gem_ptp_caps_template;
> +
> + /* nominal frequency and maximum adjustment in ppb */
> + bp->tsu_rate = bp->ptp_info->get_tsu_rate(bp);
> + bp->ptp_clock_info.max_adj = bp->ptp_info->get_ptp_max_adj();
> + gem_ptp_init_timer(bp);
> + bp->ptp_clock = ptp_clock_register(&bp->ptp_clock_info, &dev->dev);
> + if (IS_ERR(&bp->ptp_clock)) {
> + bp->ptp_clock = NULL;
> + pr_err("ptp clock register failed\n");
> + return;
> + }
> +
> + spin_lock_init(&bp->tsu_clk_lock);
> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> + queue->tx_ts_head = 0;
> + queue->tx_ts_tail = 0;
> + INIT_WORK(&queue->tx_ts_task, gem_tx_timestamp_flush);
> + queue_writel(queue, IER, MACB_PTP_INT_MASK);
> + }
> +
> + gem_ptp_init_tsu(bp);
> +
> + dev_info(&bp->pdev->dev, "%s ptp clock registered.\n",
> + GEM_PTP_TIMER_NAME);
> +}
> +
> +void gem_ptp_remove(struct net_device *ndev)
> +{
> + struct macb *bp = netdev_priv(ndev);
> +
> + if (bp->ptp_clock)
> + ptp_clock_unregister(bp->ptp_clock);
> +
> + gem_ptp_clear_timer(bp);
> +
> + dev_info(&bp->pdev->dev, "%s ptp clock unregistered.\n",
> + GEM_PTP_TIMER_NAME);
> +}
> +
> +static int gem_ptp_set_ts_mode(struct macb *bp,
> + enum macb_bd_control tx_bd_control,
> + enum macb_bd_control rx_bd_control)
> +{
> + if (!bp)
> + return -EINVAL;
Useless test.
> +
> + gem_writel(bp, TXBDCTRL, GEM_BF(TXTSMODE, tx_bd_control));
> + gem_writel(bp, RXBDCTRL, GEM_BF(RXTSMODE, rx_bd_control));
> +
> + return 0;
> +}
> +
> +int gem_get_hwtst(struct net_device *dev, struct ifreq *rq)
> +{
> + struct macb *bp = netdev_priv(dev);
> + struct hwtstamp_config *tstamp_config = &bp->tstamp_config;
> +
> + if (!bp->ptp_hw_support)
> + return -EFAULT;
> +
> + if (copy_to_user(rq->ifr_data, tstamp_config, sizeof(*tstamp_config)))
> + return -EFAULT;
> + else
> + return 0;
> +}
> +
> +static int gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
> +{
> + u32 reg_val;
> +
> + if (!bp || enable > 1)
> + return -EINVAL;
Useless test.
> +
> + reg_val = macb_readl(bp, NCR);
> +
> + if (enable)
> + macb_writel(bp, NCR, reg_val | MACB_BIT(OSSMODE));
> + else
> + macb_writel(bp, NCR, reg_val & ~MACB_BIT(OSSMODE));
> +
> + return 0;
> +}
> +
> +int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
> +{
> + struct macb *bp = netdev_priv(dev);
> + struct hwtstamp_config *tstamp_config = &bp->tstamp_config;
> + enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
> + enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
> + u32 regval;
> +
> + if (!bp->ptp_hw_support)
> + return -EFAULT;
> +
> + if (copy_from_user(tstamp_config, ifr->ifr_data,
> + sizeof(*tstamp_config)))
> + return -EFAULT;
> +
> + /* reserved for future extensions */
> + if (tstamp_config->flags)
> + return -EINVAL;
> +
> + switch (tstamp_config->tx_type) {
> + case HWTSTAMP_TX_OFF:
> + break;
> + case HWTSTAMP_TX_ONESTEP_SYNC:
> + if (gem_ptp_set_one_step_sync(bp, 1) != 0)
> + return -ERANGE;
> + case HWTSTAMP_TX_ON:
> + tx_bd_control = TSTAMP_ALL_FRAMES;
> + break;
> + default:
> + return -ERANGE;
> + }
> +
> + switch (tstamp_config->rx_filter) {
> + case HWTSTAMP_FILTER_NONE:
> + break;
> + case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> + break;
> + case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> + break;
> + case HWTSTAMP_FILTER_PTP_V2_EVENT:
> + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> + case HWTSTAMP_FILTER_PTP_V2_SYNC:
> + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
> + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
> + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> + rx_bd_control = TSTAMP_ALL_PTP_FRAMES;
> + tstamp_config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> + regval = macb_readl(bp, NCR);
> + macb_writel(bp, NCR, (regval | MACB_BIT(SRTSM)));
> + break;
> + case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> + case HWTSTAMP_FILTER_ALL:
> + rx_bd_control = TSTAMP_ALL_FRAMES;
> + tstamp_config->rx_filter = HWTSTAMP_FILTER_ALL;
> + break;
> + default:
> + tstamp_config->rx_filter = HWTSTAMP_FILTER_NONE;
> + return -ERANGE;
> + }
> +
> + if (gem_ptp_set_ts_mode(bp, tx_bd_control, rx_bd_control) != 0)
> + return -ERANGE;
> +
> + if (copy_to_user(ifr->ifr_data, tstamp_config, sizeof(*tstamp_config)))
> + return -EFAULT;
> + else
> + return 0;
> +}
> +
> +static int gem_ptp_time_peer_frame_tx_get(struct macb *bp,
> + struct timespec64 *ts)
> +{
> + if (!bp || !ts)
> + return -EINVAL;
> +
> + ts->tv_sec = (((u64)gem_readl(bp, PEFTSH) << 32) |
> + gem_readl(bp, PEFTSL)) & TSU_SEC_MAX_VAL;
> + ts->tv_nsec = gem_readl(bp, PEFTN);
> +
> + return 0;
> +}
> +
> +static int gem_ptp_time_peer_frame_rx_get(struct macb *bp,
> + struct timespec64 *ts)
> +{
> + if (!bp || !ts)
> + return -EINVAL;
> +
> + ts->tv_sec = (((u64)gem_readl(bp, PEFRSH) << 32) |
> + gem_readl(bp, PEFRSL)) & TSU_SEC_MAX_VAL;
> + ts->tv_nsec = gem_readl(bp, PEFRN);
> +
> + return 0;
> +}
> +
> +static int gem_ptp_time_frame_tx_get(struct macb *bp, struct timespec64 *ts)
> +{
> + if (!bp || !ts)
> + return -EINVAL;
> +
> + ts->tv_sec = (((u64)gem_readl(bp, EFTSH) << 32) |
> + gem_readl(bp, EFTSL)) & TSU_SEC_MAX_VAL;
> + ts->tv_nsec = gem_readl(bp, EFTN);
> +
> + return 0;
> +}
> +
> +static int gem_ptp_time_frame_rx_get(struct macb *bp, struct timespec64 *ts)
> +{
> + if (!bp || !ts)
> + return -EINVAL;
> +
> + ts->tv_sec = (((u64)gem_readl(bp, EFRSH) << 32) |
> + gem_readl(bp, EFRSL)) & TSU_SEC_MAX_VAL;
> + ts->tv_nsec = gem_readl(bp, EFRN);
> +
> + return 0;
> +}
> +
> +static int gem_ptp_event(struct macb *bp, struct timespec64 *ts)
> +{
> + struct ptp_clock_event event;
> +
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 0;
> + event.timestamp = ts->tv_sec * NSEC_PER_SEC + ts->tv_nsec;
> +
> + ptp_clock_event(bp->ptp_clock, &event);
Here you produce time stamps on external input events, but you said
that you have only one channel:
.n_ext_ts = 1,
So why do you call this function...
> +
> + return 0;
> +}
> +
> +void macb_ptp_int(struct macb_queue *queue, u32 status)
> +{
> + struct macb *bp = queue->bp;
> + struct timespec64 ts;
> +
> + if (status & MACB_BIT(DRQFR)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(DRQFR));
> + if (gem_ptp_time_frame_rx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
One ...
> + }
> +
> + if (status & MACB_BIT(SFR)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(SFR));
> + if (gem_ptp_time_frame_rx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
Two ...
> + }
> +
> + if (status & MACB_BIT(DRQFT)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(DRQFT));
> + if (gem_ptp_time_frame_tx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
Three ...
> + }
> +
> + if (status & MACB_BIT(SFT)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(SFT));
> + if (gem_ptp_time_frame_tx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
> + }
> +
> + if (status & MACB_BIT(PDRQFR)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(PDRQFR));
> + if (gem_ptp_time_peer_frame_rx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
> + }
> +
> + if (status & MACB_BIT(PDRSFR)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, MACB_BIT(PDRSFR));
> + if (gem_ptp_time_peer_frame_rx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
> + }
> +
> + if (status & MACB_BIT(PDRQFT)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR,
> + MACB_BIT(PDRQFT));
> + if (gem_ptp_time_peer_frame_tx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
> + }
> +
> + if (status & MACB_BIT(PDRSFT)) {
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR,
> + MACB_BIT(PDRSFT));
> + if (gem_ptp_time_peer_frame_tx_get(bp, &ts) != 0) {
> + ts.tv_sec = 0;
> + ts.tv_nsec = 0;
> + }
> + gem_ptp_event(bp, &ts);
.. eight times?
> + }
> +}
> --
> 2.4.5
>
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: hwmon: Add bindings for Google Chromium EC HWMON
From: Moritz Fischer @ 2017-04-14 17:47 UTC (permalink / raw)
To: Rob Herring
Cc: Guenter Roeck, Moritz Fischer, linux-hwmon,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Lee Jones, Olof Johansson, Jean Delvare, Mark Rutland
In-Reply-To: <CAL_Jsq+OaMAdRLuC0BBDpOPapisaETaUZpEX10yJk=fYLQCCdg@mail.gmail.com>
On Fri, Apr 14, 2017 at 5:48 AM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Apr 13, 2017 at 4:07 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On Thu, Apr 13, 2017 at 03:01:40PM -0500, Rob Herring wrote:
>>> On Fri, Apr 07, 2017 at 03:00:09PM -0700, Moritz Fischer wrote:
>>> > From: Moritz Fischer <mdf@kernel.org>
>>> >
>>> > Add bindings for the Chromium EC HWMON. The Chromium EC HWMON
>>> > allows monitoring of temperature sensors and fans attached to the
>>> > EC.
>>> >
>>> > Signed-off-by: Moritz Fischer <mdf@kernel.org>
>>> > ---
>>> > .../devicetree/bindings/hwmon/cros-ec-hwmon.txt | 25 ++++++++++++++++++++++
>>> > 1 file changed, 25 insertions(+)
>>> > create mode 100644 Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt b/Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt
>>> > new file mode 100644
>>> > index 0000000..4c94869
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt
>>> > @@ -0,0 +1,25 @@
>>> > +Chromium Embedded Controller EC temperature and fan control
>>> > +-----------------------------------------------------------
>>> > +
>>> > +Google's Chromium EC HWMON is a hwmon implemented byimplemented by the Chromium EC
>>> > +firmware attached to the Embedded Controller (EC) and controlled via a host-command
>>> > +interface.
>>> > +
>>> > +An EC HWMON node should be only found as a sub-node of the EC node (see
>>> > +Documentation/devicetree/bindings/mfd/cros-ec.txt).
>>> > +
>>> > +Required properties:
>>> > +- compatible: Must contain "google,cros-ec-hwmon"
>>> > +
>>> > +Example:
>>> > + embedded-controller@1e {
>>> > + reg = <0x1e>;
>>> > + compatible = "google,cros-ec-i2c";
>>> > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>>> > + interrupt-parent = <&gpio0>;
>>> > +
>>> > + hwmon {
>>> > + compatible = "google,cros-ec-hwmon";
>>>
>>> This is sufficient for all devices? I don't see that DT provides
>>> anything here other than instantiating a device, but the parent device
>>> can just as easily do that.
>>>
>> The parent driver (drivers/mfd/cros_ec_i2c.c) calls cros_ec_register(),
>> which uses uses of_platform_populate() to populate all sub-devices.
>> There are various examples in the dts files (look for "google,cros-ec").
>> Does it really make sense to start a second method for instantiating
>> sub-devices ?
>
> Okay, I suppose not. That wasn't clear from the example.
Do you want me to clarify that in the example somehow?
Moritz
^ permalink raw reply
* [PATCH] arm64: dts: allwinner: a64: Add UART2 pin nodes
From: Andreas Färber @ 2017-04-14 17:13 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Andreas Färber, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
Mark Rutland, Catalin Marinas, Will Deacon, devicetree,
linux-kernel
UART2 is exposed on the Pi connector of Pine64. Make a pinctrl node
available at the SoC level, to simplify enabling UART2 via DT overlay.
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 1c64ea2d23f9..61d3a19659bf 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -285,6 +285,11 @@
pins = "PG8", "PG9";
function = "uart1";
};
+
+ uart2_pins: uart2-pins {
+ pins = "PB0", "PB1";
+ function = "uart2";
+ };
};
uart0: serial@1c28000 {
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v2 11/11] arm64: allwinner: a64: enable Wi-Fi for Pine64
From: Andreas Färber @ 2017-04-14 16:56 UTC (permalink / raw)
To: Icenowy Zheng, Lee Jones, Rob Herring, Chen-Yu Tsai,
Maxime Ripard, Liam Girdwood
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170407183441.58750-12-icenowy-h8G6r0blFSE@public.gmane.org>
Hi,
Am 07.04.2017 um 20:34 schrieb Icenowy Zheng:
> The Wi-Fi modules of Pine64 is powered via DLDO4 and ELDO1 (the latter
"modules ... are" or "module ... is"
> one provides I/O voltage).
>
> Add device node for it.
>
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
> arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> index 7da074f95065..9d90bb32aa87 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> @@ -64,6 +64,11 @@
> regulator-min-microvolt = <3300000>;
> regulator-max-microvolt = <3300000>;
> };
> +
> + wifi_pwrseq: wifi_pwrseq {
> + compatible = "mmc-pwrseq-simple";
> + reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
> + };
> };
>
> &ehci1 {
> @@ -91,6 +96,17 @@
> status = "okay";
> };
>
> +&mmc1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc1_pins>;
> + vmmc-supply = <®_dldo4>;
> + vqmmc-supply = <®_eldo1>;
> + mmc-pwrseq = <&wifi_pwrseq>;
> + non-removable;
> + bus-width = <4>;
> + status = "okay";
> +};
As you mention above, Wi-Fi is a module, so may be absent. Is it really
correct to enable this node and hardcode a certain power sequence? In
theory other modules could be attached.
To me that calls for an overlay instead.
> +
> &ohci1 {
> status = "okay";
> };
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 5/8] ARM: dts: imx7s: Mark 'gpr' compatible with i.MX6 variant
From: Andrey Smirnov @ 2017-04-14 16:30 UTC (permalink / raw)
To: Dong Aisheng
Cc: Shawn Guo, Andrey Yurovsky, Sascha Hauer, Fabio Estevam,
Rob Herring, Mark Rutland, Russell King,
devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel, linux-arm-kernel
In-Reply-To: <20170414155639.GC1792@b29396-OptiPlex-7040>
On Fri, Apr 14, 2017 at 8:56 AM, Dong Aisheng <dongas86-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Apr 13, 2017 at 06:32:39AM -0700, Andrey Smirnov wrote:
>> List GPR block as compatible "fsl,imx6q-iomuxc-gpr" to support drivers
>> requesting it that way (PCIe driver is one example).
>>
>> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 1a7058f..cc23478 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -491,7 +491,8 @@
>> };
>>
>> gpr: iomuxc-gpr@30340000 {
>> - compatible = "fsl,imx7d-iomuxc-gpr", "syscon";
>> + compatible = "fsl,imx7d-iomuxc-gpr",
>> + "fsl,imx6q-iomuxc-gpr", "syscon";
>
> This looks wrong to me.
> mx7d-iomux-gpr gets a big difference from mx6q-iomux-gpr and mostly
> not compatible.
>
AFAICT, there are no upstream drivers that bind to that string
directly and all of the "consumers" of this node request it as a
syscon device. The only code I could find that does so and that is
shared between i.MX7 and i.MX6Q is i.MX PCIe driver which
distinguishes between variants based on its own compatibility string.
Those two register files are different, true, but I don't think there
are any users who try to use them as if they were the same/compatible.
Thanks,
Andrey Smirnov
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Marek Vasut @ 2017-04-14 16:15 UTC (permalink / raw)
To: Ludovic Barre, Cyrille Pitchen
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Alexandre Torgue, Rob Herring,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492103757-22375-3-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>
On 04/13/2017 07:15 PM, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>
> The quadspi is a specialized communication interface targeting single,
> dual or quad SPI Flash memories.
>
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
> registers
> -read memory-mapped mode: the external Flash memory is mapped to the
> microcontroller address space and is seen by the system as if it was
> an internal memory
>
> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
Reviewed-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
From: Andrey Smirnov @ 2017-04-14 16:08 UTC (permalink / raw)
To: Dong Aisheng
Cc: Shawn Guo, Andrey Yurovsky, Sascha Hauer, Fabio Estevam,
Rob Herring, Mark Rutland, Russell King,
devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel, linux-arm-kernel
In-Reply-To: <20170414153243.GA1792@b29396-OptiPlex-7040>
On Fri, Apr 14, 2017 at 8:32 AM, Dong Aisheng <dongas86-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Apr 13, 2017 at 06:32:37AM -0700, Andrey Smirnov wrote:
>> In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
>> it serves the function of granting permission to GPC IP block to alter
>> various bit-fields of the register. The reason why this property, that
>> trickeld here from Freescale BSP, is set to 31 is because in the code
>> it came from it is used in conjunction with a notifier handler for
>> REGULATOR_EVENT_PRE_DO_ENABLE and REGULATOR_EVENT_PRE_DO_DISABLE
>> events (not found in upstream kernel) that triggers GPC to start
>> manipulating aforementioned other bitfields.
>>
>> Since:
>> a) none of the aforementioned machinery is implemented by
>> upstream
>> b) using 'anatop-enable-bit' in that capacity is a bit of a
>> semantic stretch
>
> Yes, this does is a bit of semantic stretch.
> FSL using is combined with regulator notify and that do bring a bit
> of complexity.
>
> I'm not sure if it's good to introduce another anatop-override-bit
> to separate, but i'm a bit scare since there's already many....
>
All of those Freescale specific events are replaced by GPCv2 power
domain driver that we discussed in another thread. Since regulator
driver for ANADIG sets up all of the voltages manually (or, more
specifically, GPCv2 driver sets them up via regulator API) I didn't
see any reason to use OVERRIDE instead of just ENABLE.
>From reading the RM it seems that main reason for using OVERRIDE as
opposed to ENABLE would be to leverage advanced hardware power
management capabilities of the SoC which I don't think are implemented
in upstream kernel. Do you think there's a use-case for
anatop-override-bit property?
>>
>> simplify the situation by setting the value of 'anatop-enable-bit' to
>> point to ENABLE_LINREG (same as i.MX6).
>>
>> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 22c9788..8fee299 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -516,7 +516,7 @@
>> anatop-min-bit-val = <8>;
>> anatop-min-voltage = <800000>;
>> anatop-max-voltage = <1200000>;
>> - anatop-enable-bit = <31>;
>> + anatop-enable-bit = <0>;
>
> The change of this line seems already exist in patch 1.
I am going to squash all three patches into a single one.
Thanks,
Andrey Smirnov
--
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
* 24401 devicetree
From: sushildhimanphotography-/E1597aS9LQAvxtiuMwx3w @ 2017-04-14 16:06 UTC (permalink / raw)
To: devicetree
[-- Attachment #1: 942108090439.zip --]
[-- Type: application/zip, Size: 2140 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] ARM: dts: imx7d-sdb: Add GPIO expander node
From: Dong Aisheng @ 2017-04-14 16:00 UTC (permalink / raw)
To: Shawn Guo
Cc: Andrey Smirnov, yurovsky, Sascha Hauer, Fabio Estevam,
Rob Herring, Mark Rutland, Russell King, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20170414034659.GH14915@dragon>
On Fri, Apr 14, 2017 at 11:47:00AM +0800, Shawn Guo wrote:
> On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote:
> > Add node for U38, a 74LV595PW serial-in shift register that acts as a
> > GPIO expander on the board.
> >
> > Cc: yurovsky@gmail.com
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> > arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
> > index 5be01a1..e0ff276 100644
> > --- a/arch/arm/boot/dts/imx7d-sdb.dts
> > +++ b/arch/arm/boot/dts/imx7d-sdb.dts
> > @@ -52,6 +52,30 @@
> > reg = <0x80000000 0x80000000>;
> > };
> >
> > + spi4 {
> > + compatible = "spi-gpio";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_spi1>;
> > + status = "okay";
>
> The 'status' is not needed in this case.
>
> > + gpio-sck = <&gpio1 13 0>;
> > + gpio-mosi = <&gpio1 9 0>;
> > + cs-gpios = <&gpio1 12 0>;
> > + num-chipselects = <1>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + gpio_spi: gpio_spi@0 {
>
> gpio-expander might be a better node name?
>
> > + compatible = "fairchild,74hc595";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + reg = <0>;
> > + registers-number = <1>;
> > + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/
> > + registers-default = /bits/ 8 <0x74>;
>
> I do not see this property is documented or supported by kernel.
It's FSL internal invented property to do some trick on register
intialization and should be dropped.
Regards
Dong Aisheng
^ permalink raw reply
* Re: [PATCH 5/8] ARM: dts: imx7s: Mark 'gpr' compatible with i.MX6 variant
From: Dong Aisheng @ 2017-04-14 15:56 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Shawn Guo, yurovsky, Sascha Hauer, Fabio Estevam, Rob Herring,
Mark Rutland, Russell King, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20170413133242.5068-6-andrew.smirnov@gmail.com>
On Thu, Apr 13, 2017 at 06:32:39AM -0700, Andrey Smirnov wrote:
> List GPR block as compatible "fsl,imx6q-iomuxc-gpr" to support drivers
> requesting it that way (PCIe driver is one example).
>
> Cc: yurovsky@gmail.com
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> arch/arm/boot/dts/imx7s.dtsi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 1a7058f..cc23478 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -491,7 +491,8 @@
> };
>
> gpr: iomuxc-gpr@30340000 {
> - compatible = "fsl,imx7d-iomuxc-gpr", "syscon";
> + compatible = "fsl,imx7d-iomuxc-gpr",
> + "fsl,imx6q-iomuxc-gpr", "syscon";
This looks wrong to me.
mx7d-iomux-gpr gets a big difference from mx6q-iomux-gpr and mostly
not compatible.
Regards
Dong Aisheng
^ permalink raw reply
* Re: [PATCH 4/8] ARM: dts: imx7s: Add node for GPC
From: Andrey Smirnov @ 2017-04-14 15:50 UTC (permalink / raw)
To: Dong Aisheng
Cc: Shawn Guo, Andrey Yurovsky, Sascha Hauer, Fabio Estevam,
Rob Herring, Mark Rutland, Russell King,
devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel, linux-arm-kernel
In-Reply-To: <20170414154936.GB1792@b29396-OptiPlex-7040>
On Fri, Apr 14, 2017 at 8:49 AM, Dong Aisheng <dongas86-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Apr 14, 2017 at 08:19:44AM -0700, Andrey Smirnov wrote:
> ...
>> >> + gpc: gpc@303a0000 {
>> >> + compatible = "fsl,imx7d-gpc";
>> >> + reg = <0x303a0000 0x10000>;
>> >> + interrupt-controller;
>> >> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>> >> + #interrupt-cells = <3>;
>> >> + interrupt-parent = <&intc>;
>> >> + #power-domain-cells = <1>;
>> >> +
>> >> + pgc {
>> >> + #address-cells = <1>;
>> >> + #size-cells = <0>;
>> >> +
>> >> + pgc_pcie_phy: pgc-pcie-phy-domain {
>> >
>> > The node name should be something generic and has a unit-address when
>> > there is a 'reg' property in the node.
>> >
>>
>> I'll change it to pgc-power-domain@0, let me know if you want
>> something different.
>>
>
> I think just power-domain@0 is ok.
Fair enough. I'll do that.
> And also better replace unit-address by macro.
>
Good point. Will do.
Thanks,
Andrey Smirnov
--
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 4/8] ARM: dts: imx7s: Add node for GPC
From: Dong Aisheng @ 2017-04-14 15:49 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Shawn Guo, Andrey Yurovsky, Sascha Hauer, Fabio Estevam,
Rob Herring, Mark Rutland, Russell King,
devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel, linux-arm-kernel
In-Reply-To: <CAHQ1cqGYs2h1ZFBC4WBYbyAMsctURwOzQmQB+B6nz=wmR0fYoQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Apr 14, 2017 at 08:19:44AM -0700, Andrey Smirnov wrote:
...
> >> + gpc: gpc@303a0000 {
> >> + compatible = "fsl,imx7d-gpc";
> >> + reg = <0x303a0000 0x10000>;
> >> + interrupt-controller;
> >> + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> >> + #interrupt-cells = <3>;
> >> + interrupt-parent = <&intc>;
> >> + #power-domain-cells = <1>;
> >> +
> >> + pgc {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + pgc_pcie_phy: pgc-pcie-phy-domain {
> >
> > The node name should be something generic and has a unit-address when
> > there is a 'reg' property in the node.
> >
>
> I'll change it to pgc-power-domain@0, let me know if you want
> something different.
>
I think just power-domain@0 is ok.
And also better replace unit-address by macro.
Regards
Dong Aisheng
--
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 8/8] ARM: dts: imx7d-sdb: Enable PCIe peripheral
From: Andrey Smirnov @ 2017-04-14 15:45 UTC (permalink / raw)
To: Shawn Guo
Cc: Andrey Yurovsky, Sascha Hauer, Fabio Estevam, Rob Herring,
Mark Rutland, Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel, linux-arm-kernel
In-Reply-To: <20170414035102.GI14915@dragon>
On Thu, Apr 13, 2017 at 8:51 PM, Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Apr 13, 2017 at 06:32:42AM -0700, Andrey Smirnov wrote:
>> Enable PCIe peripheral on this board.
>>
>> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/imx7d-sdb.dts | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
>> index e0ff276..f77e26a 100644
>> --- a/arch/arm/boot/dts/imx7d-sdb.dts
>> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
>> @@ -352,6 +352,13 @@
>> };
>> };
>>
>> +&pcie {
>> + pinctrl-names = "default";
>> + reset-gpio = <&gpio_spi 1 GPIO_ACTIVE_LOW>;
>> + disable-gpio = <&gpio_spi 0 GPIO_ACTIVE_LOW>;
>
> I do not see this disable-gpio is documented or supported.
>
My bad, didn't notice it when taking the code from downstream tree.
Will remove in v2.
Thanks,
Andrey Smirnov
--
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 v6 4/4] ARM: dts: armada-xp: Use pwm-fan rather than gpio-fan
From: Ralph Sennhauser @ 2017-04-14 15:40 UTC (permalink / raw)
To: linux-gpio-u79uwXL29TY76Z2rM5mHXA
Cc: Andrew Lunn, Ralph Sennhauser, Thierry Reding, Linus Walleij,
Alexandre Courbot, Rob Herring, Mark Rutland, Jason Cooper,
Gregory Clement, Sebastian Hesselbarth, Russell King,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170414154056.32055-1-ralph.sennhauser-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
The mvebu GPIO driver can also perform PWM on some pins. Use the pwm-fan
driver to control the fan of the WRT1900AC, giving us finer grained control
over its speed and hence noise.
Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
URL: https://patchwork.ozlabs.org/patch/427291/
[Ralph Sennhauser: drop flags paramter from pwms, no longer used]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
---
arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
index 9efcf59..6d705f5 100644
--- a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
+++ b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
@@ -308,13 +308,11 @@
};
};
- gpio_fan {
+ pwm_fan {
/* SUNON HA4010V4-0000-C99 */
- compatible = "gpio-fan";
- gpios = <&gpio0 24 0>;
- gpio-fan,speed-map = <0 0
- 4500 1>;
+ compatible = "pwm-fan";
+ pwms = <&gpio0 24 4000>;
};
dsa {
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v6 3/4] ARM: mvebu: Enable SENSORS_PWM_FAN in defconfig
From: Ralph Sennhauser @ 2017-04-14 15:40 UTC (permalink / raw)
To: linux-gpio
Cc: Andrew Lunn, Ralph Sennhauser, Thierry Reding, Linus Walleij,
Alexandre Courbot, Rob Herring, Mark Rutland, Jason Cooper,
Gregory Clement, Sebastian Hesselbarth, Russell King, linux-pwm,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170414154056.32055-1-ralph.sennhauser@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
Now that the GPIO driver also supports PWM operation, enable the PWM
framework and fan driver in mvebu_v7_defconfig.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
URL: https://patchwork.ozlabs.org/patch/427297/
[Ralph Sennhauser: add fan driver to defconfig]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/configs/mvebu_v7_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig
index f1a0e25..6955370 100644
--- a/arch/arm/configs/mvebu_v7_defconfig
+++ b/arch/arm/configs/mvebu_v7_defconfig
@@ -135,6 +135,8 @@ CONFIG_DMADEVICES=y
CONFIG_MV_XOR=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_MEMORY=y
+CONFIG_PWM=y
+CONFIG_SENSORS_PWM_FAN=y
CONFIG_EXT4_FS=y
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
--
2.10.2
^ permalink raw reply related
* [PATCH v6 2/4] ARM: dts: mvebu: Add PWM properties to .dtsi files
From: Ralph Sennhauser @ 2017-04-14 15:40 UTC (permalink / raw)
To: linux-gpio
Cc: Andrew Lunn, Ralph Sennhauser, Thierry Reding, Linus Walleij,
Alexandre Courbot, Rob Herring, Mark Rutland, Jason Cooper,
Gregory Clement, Sebastian Hesselbarth, Russell King, linux-pwm,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170414154056.32055-1-ralph.sennhauser@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
Add properties to the GPIO nodes to allow them to be also used as PWM
lines.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
URL: https://patchwork.ozlabs.org/patch/427294/
[Ralph Sennhauser: Add new compatible string marvell,armada-370-xp-gpio]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/boot/dts/armada-370.dtsi | 19 ++++++++++++++-----
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 16 ++++++++++++----
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 19 ++++++++++++++-----
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 19 ++++++++++++++-----
4 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index cc011c8..5e815cc 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -137,29 +137,38 @@
};
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
- compatible = "marvell,orion-gpio";
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
reg = <0x18180 0x40>;
ngpios = <2>;
gpio-controller;
diff --git a/arch/arm/boot/dts/armada-xp-mv78230.dtsi b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
index 07c5090..f77168c9 100644
--- a/arch/arm/boot/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
@@ -202,25 +202,33 @@
internal-regs {
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <17>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
};
};
};
diff --git a/arch/arm/boot/dts/armada-xp-mv78260.dtsi b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
index 64e936a..0ecfaf4 100644
--- a/arch/arm/boot/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
@@ -285,29 +285,38 @@
internal-regs {
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
- compatible = "marvell,orion-gpio";
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
reg = <0x18180 0x40>;
ngpios = <3>;
gpio-controller;
diff --git a/arch/arm/boot/dts/armada-xp-mv78460.dtsi b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
index d1383dd..670ece4c 100644
--- a/arch/arm/boot/dts/armada-xp-mv78460.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
@@ -323,29 +323,38 @@
internal-regs {
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
- compatible = "marvell,orion-gpio";
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
reg = <0x18180 0x40>;
ngpios = <3>;
gpio-controller;
--
2.10.2
^ permalink raw reply related
* [PATCH v6 1/4] gpio: mvebu: Add limited PWM support
From: Ralph Sennhauser @ 2017-04-14 15:40 UTC (permalink / raw)
To: linux-gpio
Cc: Andrew Lunn, Ralph Sennhauser, Thierry Reding, Linus Walleij,
Alexandre Courbot, Rob Herring, Mark Rutland, Jason Cooper,
Gregory Clement, Sebastian Hesselbarth, Russell King, linux-pwm,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170414154056.32055-1-ralph.sennhauser@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
Armada 370/XP devices can 'blink' GPIO lines with a configurable on
and off period. This can be modelled as a PWM.
However, there are only two sets of PWM configuration registers for
all the GPIO lines. This driver simply allows a single GPIO line per
GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
EBUSY.
Due to the interleaving of registers it is not simple to separate the
PWM driver from the GPIO driver. Thus the GPIO driver has been
extended with a PWM driver.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
URL: https://patchwork.ozlabs.org/patch/427287/
URL: https://patchwork.ozlabs.org/patch/427295/
[Ralph Sennhauser:
* Port forward
* Merge PWM portion into gpio-mvebu.c
* Switch to atomic PWM API
* Add new compatible string marvell,armada-370-xp-gpio
* Update and merge documentation patch
* Update MAINTAINERS]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
MAINTAINERS | 2 +
drivers/gpio/gpio-mvebu.c | 327 ++++++++++++++++++++-
3 files changed, 349 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
index a6f3bec..42c3bb2 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
@@ -38,6 +38,24 @@ Required properties:
- #gpio-cells: Should be two. The first cell is the pin number. The
second cell is reserved for flags, unused at the moment.
+Optional properties:
+
+In order to use the GPIO lines in PWM mode, some additional optional
+properties are required. Only Armada 370 and XP support these properties.
+
+- compatible: Must contain "marvell,armada-370-xp-gpio"
+
+- reg: an additional register set is needed, for the GPIO Blink
+ Counter on/off registers.
+
+- reg-names: Must contain an entry "pwm" corresponding to the
+ additional register range needed for PWM operation.
+
+- #pwm-cells: Should be two. The first cell is the GPIO line number. The
+ second cell is the period in nanoseconds.
+
+- clocks: Must be a phandle to the clock for the GPIO controller.
+
Example:
gpio0: gpio@d0018100 {
@@ -51,3 +69,17 @@ Example:
#interrupt-cells = <2>;
interrupts = <16>, <17>, <18>, <19>;
};
+
+ gpio1: gpio@18140 {
+ compatible = "marvell,armada-370-xp-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
+ ngpios = <17>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #pwm-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 58b3a22..19382f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10295,6 +10295,8 @@ F: include/linux/pwm.h
F: drivers/pwm/
F: drivers/video/backlight/pwm_bl.c
F: include/linux/pwm_backlight.h
+F: drivers/gpio/gpio-mvebu.c
+F: Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
PXA2xx/PXA3xx SUPPORT
M: Daniel Mack <daniel@zonque.org>
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index fae4db6..19a92ef 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -42,22 +42,34 @@
#include <linux/io.h>
#include <linux/of_irq.h>
#include <linux/of_device.h>
+#include <linux/pwm.h>
#include <linux/clk.h>
#include <linux/pinctrl/consumer.h>
#include <linux/irqchip/chained_irq.h>
+#include <linux/platform_device.h>
#include <linux/bitops.h>
+#include "gpiolib.h"
+
/*
* GPIO unit register offsets.
*/
-#define GPIO_OUT_OFF 0x0000
-#define GPIO_IO_CONF_OFF 0x0004
-#define GPIO_BLINK_EN_OFF 0x0008
-#define GPIO_IN_POL_OFF 0x000c
-#define GPIO_DATA_IN_OFF 0x0010
-#define GPIO_EDGE_CAUSE_OFF 0x0014
-#define GPIO_EDGE_MASK_OFF 0x0018
-#define GPIO_LEVEL_MASK_OFF 0x001c
+#define GPIO_OUT_OFF 0x0000
+#define GPIO_IO_CONF_OFF 0x0004
+#define GPIO_BLINK_EN_OFF 0x0008
+#define GPIO_IN_POL_OFF 0x000c
+#define GPIO_DATA_IN_OFF 0x0010
+#define GPIO_EDGE_CAUSE_OFF 0x0014
+#define GPIO_EDGE_MASK_OFF 0x0018
+#define GPIO_LEVEL_MASK_OFF 0x001c
+#define GPIO_BLINK_CNT_SELECT_OFF 0x0020
+
+/*
+ * PWM register offsets.
+ */
+#define PWM_BLINK_ON_DURATION_OFF 0x0
+#define PWM_BLINK_OFF_DURATION_OFF 0x4
+
/* The MV78200 has per-CPU registers for edge mask and level mask */
#define GPIO_EDGE_MASK_MV78200_OFF(cpu) ((cpu) ? 0x30 : 0x18)
@@ -78,6 +90,20 @@
#define MVEBU_MAX_GPIO_PER_BANK 32
+struct mvebu_pwm {
+ void __iomem *membase;
+ unsigned long clk_rate;
+ struct gpio_desc *gpiod;
+ struct pwm_chip chip;
+ spinlock_t lock;
+ struct mvebu_gpio_chip *mvchip;
+
+ /* Used to preserve GPIO/PWM registers across suspend/resume */
+ u32 blink_select;
+ u32 blink_on_duration;
+ u32 blink_off_duration;
+};
+
struct mvebu_gpio_chip {
struct gpio_chip chip;
spinlock_t lock;
@@ -87,6 +113,10 @@ struct mvebu_gpio_chip {
struct irq_domain *domain;
int soc_variant;
+ /* Used for PWM support */
+ struct clk *clk;
+ struct mvebu_pwm *mvpwm;
+
/* Used to preserve GPIO registers across suspend/resume */
u32 out_reg;
u32 io_conf_reg;
@@ -110,6 +140,12 @@ static void __iomem *mvebu_gpioreg_blink(struct mvebu_gpio_chip *mvchip)
return mvchip->membase + GPIO_BLINK_EN_OFF;
}
+static void __iomem *mvebu_gpioreg_blink_counter_select(struct mvebu_gpio_chip
+ *mvchip)
+{
+ return mvchip->membase + GPIO_BLINK_CNT_SELECT_OFF;
+}
+
static void __iomem *mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
{
return mvchip->membase + GPIO_IO_CONF_OFF;
@@ -181,6 +217,20 @@ static void __iomem *mvebu_gpioreg_level_mask(struct mvebu_gpio_chip *mvchip)
}
/*
+ * Functions returning addresses of individual registers for a given
+ * PWM controller.
+ */
+static void __iomem *mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *mvpwm)
+{
+ return mvpwm->membase + PWM_BLINK_ON_DURATION_OFF;
+}
+
+static void __iomem *mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)
+{
+ return mvpwm->membase + PWM_BLINK_OFF_DURATION_OFF;
+}
+
+/*
* Functions implementing the gpio_chip methods
*/
static void mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
@@ -484,6 +534,246 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
+/*
+ * Functions implementing the pwm_chip methods
+ */
+static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
+{
+ return container_of(chip, struct mvebu_pwm, chip);
+}
+
+static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ struct gpio_desc *desc;
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ if (mvpwm->gpiod) {
+ ret = -EBUSY;
+ } else {
+ desc = gpio_to_desc(mvchip->chip.base + pwm->hwpwm);
+ if (!desc) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ ret = gpiod_request(desc, "mvebu-pwm");
+ if (ret)
+ goto out;
+
+ ret = gpiod_direction_output(desc, 0);
+ if (ret) {
+ gpiod_free(desc);
+ goto out;
+ }
+
+ mvpwm->gpiod = desc;
+ }
+out:
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+ return ret;
+}
+
+static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ unsigned long flags;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+ gpiod_free(mvpwm->gpiod);
+ mvpwm->gpiod = NULL;
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+}
+
+static void mvebu_pwm_get_state(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ struct pwm_state *state) {
+
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ unsigned long long val;
+ unsigned long flags;
+ u32 u;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ val = (unsigned long long)
+ readl_relaxed(mvebu_pwmreg_blink_on_duration(mvpwm));
+ val *= NSEC_PER_SEC;
+ do_div(val, mvpwm->clk_rate);
+ if (val > UINT_MAX)
+ state->duty_cycle = UINT_MAX;
+ else if (val)
+ state->duty_cycle = val;
+ else
+ state->duty_cycle = 1;
+
+ val = (unsigned long long)
+ readl_relaxed(mvebu_pwmreg_blink_off_duration(mvpwm));
+ val *= NSEC_PER_SEC;
+ do_div(val, mvpwm->clk_rate);
+ if (val < state->duty_cycle) {
+ state->period = 1;
+ } else {
+ val -= state->duty_cycle;
+ if (val > UINT_MAX)
+ state->period = UINT_MAX;
+ else if (val)
+ state->period = val;
+ else
+ state->period = 1;
+ }
+
+ u = readl_relaxed(mvebu_gpioreg_blink(mvchip));
+ if (u)
+ state->enabled = true;
+ else
+ state->enabled = false;
+
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+}
+
+static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ unsigned long long val;
+ unsigned long flags;
+ unsigned int on, off;
+
+ val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ on = val;
+ else
+ on = 1;
+
+ val = (unsigned long long) mvpwm->clk_rate *
+ (state->period - state->duty_cycle);
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ off = val;
+ else
+ off = 1;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ writel_relaxed(on, mvebu_pwmreg_blink_on_duration(mvpwm));
+ writel_relaxed(off, mvebu_pwmreg_blink_off_duration(mvpwm));
+ if (state->enabled)
+ mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 1);
+ else
+ mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 0);
+
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+
+ return 0;
+}
+
+static const struct pwm_ops mvebu_pwm_ops = {
+ .request = mvebu_pwm_request,
+ .free = mvebu_pwm_free,
+ .get_state = mvebu_pwm_get_state,
+ .apply = mvebu_pwm_apply,
+ .owner = THIS_MODULE,
+};
+
+static void __maybe_unused mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *mvpwm = mvchip->mvpwm;
+
+ mvpwm->blink_select =
+ readl_relaxed(mvebu_gpioreg_blink_counter_select(mvchip));
+ mvpwm->blink_on_duration =
+ readl_relaxed(mvebu_pwmreg_blink_on_duration(mvpwm));
+ mvpwm->blink_off_duration =
+ readl_relaxed(mvebu_pwmreg_blink_off_duration(mvpwm));
+}
+
+static void __maybe_unused mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *mvpwm = mvchip->mvpwm;
+
+ writel_relaxed(mvpwm->blink_select,
+ mvebu_gpioreg_blink_counter_select(mvchip));
+ writel_relaxed(mvpwm->blink_on_duration,
+ mvebu_pwmreg_blink_on_duration(mvpwm));
+ writel_relaxed(mvpwm->blink_off_duration,
+ mvebu_pwmreg_blink_off_duration(mvpwm));
+}
+
+static int mvebu_pwm_probe(struct platform_device *pdev,
+ struct mvebu_gpio_chip *mvchip,
+ int id)
+{
+ struct device *dev = &pdev->dev;
+ struct mvebu_pwm *mvpwm;
+ struct resource *res;
+ u32 set;
+
+ if (!of_device_is_compatible(mvchip->chip.of_node,
+ "marvell,armada-370-xp-gpio"))
+ return 0;
+
+ if (IS_ERR(mvchip->clk))
+ return PTR_ERR(mvchip->clk);
+
+ /*
+ * There are only two sets of PWM configuration registers for
+ * all the GPIO lines on those SoCs which this driver reserves
+ * for the first two GPIO chips. So if the resource is missing
+ * we can't treat it as an error.
+ */
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm");
+ if (!res)
+ return 0;
+
+ /*
+ * Use set A for lines of GPIO chip with id 0, B for GPIO chip
+ * with id 1. Don't allow further GPIO chips to be used for PWM.
+ */
+ if (id == 0)
+ set = 0;
+ else if (id == 1)
+ set = U32_MAX;
+ else
+ return -EINVAL;
+ writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
+
+ mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
+ if (!mvpwm)
+ return -ENOMEM;
+ mvchip->mvpwm = mvpwm;
+ mvpwm->mvchip = mvchip;
+
+ mvpwm->membase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mvpwm->membase))
+ return PTR_ERR(mvpwm->membase);
+
+ mvpwm->clk_rate = clk_get_rate(mvchip->clk);
+ if (!mvpwm->clk_rate) {
+ dev_err(dev, "failed to get clock rate\n");
+ return -EINVAL;
+ }
+
+ mvpwm->chip.dev = dev;
+ mvpwm->chip.ops = &mvebu_pwm_ops;
+ mvpwm->chip.npwm = mvchip->chip.ngpio;
+
+ spin_lock_init(&mvpwm->lock);
+
+ return pwmchip_add(&mvpwm->chip);
+}
+
#ifdef CONFIG_DEBUG_FS
#include <linux/seq_file.h>
@@ -555,6 +845,10 @@ static const struct of_device_id mvebu_gpio_of_match[] = {
.data = (void *) MVEBU_GPIO_SOC_VARIANT_ARMADAXP,
},
{
+ .compatible = "marvell,armada-370-xp-gpio",
+ .data = (void *) MVEBU_GPIO_SOC_VARIANT_ORION,
+ },
+ {
/* sentinel */
},
};
@@ -600,6 +894,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
BUG();
}
+ if (IS_ENABLED(CONFIG_PWM))
+ mvebu_pwm_suspend(mvchip);
+
return 0;
}
@@ -643,6 +940,9 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
BUG();
}
+ if (IS_ENABLED(CONFIG_PWM))
+ mvebu_pwm_resume(mvchip);
+
return 0;
}
@@ -654,7 +954,6 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
struct resource *res;
struct irq_chip_generic *gc;
struct irq_chip_type *ct;
- struct clk *clk;
unsigned int ngpios;
bool have_irqs;
int soc_variant;
@@ -688,10 +987,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
return id;
}
- clk = devm_clk_get(&pdev->dev, NULL);
+ mvchip->clk = devm_clk_get(&pdev->dev, NULL);
/* Not all SoCs require a clock.*/
- if (!IS_ERR(clk))
- clk_prepare_enable(clk);
+ if (!IS_ERR(mvchip->clk))
+ clk_prepare_enable(mvchip->clk);
mvchip->soc_variant = soc_variant;
mvchip->chip.label = dev_name(&pdev->dev);
@@ -822,6 +1121,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
mvchip);
}
+ /* Armada 370/XP has simple PWM support for GPIO lines */
+ if (IS_ENABLED(CONFIG_PWM))
+ return mvebu_pwm_probe(pdev, mvchip, id);
+
return 0;
err_domain:
--
2.10.2
^ permalink raw reply related
* [PATCH v5 0/4] gpio: mvebu: Add PWM fan support
From: Ralph Sennhauser @ 2017-04-14 15:40 UTC (permalink / raw)
To: linux-gpio
Cc: Ralph Sennhauser, Thierry Reding, Linus Walleij,
Alexandre Courbot, Rob Herring, Mark Rutland, Jason Cooper,
Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, Russell King,
linux-pwm, devicetree, linux-kernel, linux-arm-kernel
Hi everyone,
With the bike shedding fixes as Thierry called them out of the way and
the ACK by Rob for the bindings this version could be merged.
The only thing still sort of open is whether to use npwm=1 and giving up
the sysfs use case. Though an ACK was given for this implementation
already I think we should give Thierry a chance to speak up once more if
he so wishes.
Thanks
Ralph
---
Notes:
About npwm = 1:
The only way I can think of to achieve that requires reading the
GPIO line from the device tree. This would prevent a user to
dynamically choose a line. Which is fine for the fan found on Mamba
but let's take some development board with freely accessible GPIOs
and suddenly we limit the use of this driver (sysfs). Given the
above, npwm = ngpio with only one usable at a time is a closer /
more appropriate description of the situation. The downside is
some "wasted" space and not meeting the expectation that all PWMs
are usable concurrently, only one per chip at any given time.
About the new compatible string:
Orion was chosen for the SoC variant for the same reason as in
commit 5f79c651e81e ("arm: mvebu: use global interrupts for GPIOs on
Armada XP").
The "pwm" property remains optional for the new compatible string so
the compatiple string "marvell,armada-370-xp-gpio" can be used by
all and not just the first two GPIO chips.
About extending the driver for use with a third GPIO chip:
It's possible to add an extra node "mvebugpiopwmctrl" to configure a
different assignment without requiring changes to the current
bindings. The current implementation doesn't prevent a fan to be
used as gpio-fan connect to the 3rd chip.
---
Changes v5->v6:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* more uppercase for GPIO / PWM in prose (suggested by Thierry
Reding)
* use a variable for register set A/B selection value (suggested by
Thierry Reding)
* Don't set pwm_chip base to gpio_chip base, use hwpwm instead.
(suggested by Thierry Reding)
* use pointer to gpio_desc instead of boolen used (suggested by
Thierry Reding)
* check a bit earlier for working clock (suggested by Thierry
Reding)
* Add Acked-by: Thierry Reding <thierry.reding@gmail.com>
* Acked-by: Rob Herring <robh@kernel.org>
Changes v4->v5:
All
* add Tested-by: Andrew Lunn <andrew@lunn.ch>, thanks
Patch 2/4 mvebu: xp: Add PWM properties to .dtsi files
* keep the old compatible stings, we don't have to drop them (suggested by Gregory CLEMENT)
* subject starts with ARM: dts: mvebu: (suggested by Gregory CLEMENT)
Patch 4/4 mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
* subject starts with ARM: dts: armada-xp: (suggested by Gregory CLEMENT)
Changes v3->v4:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* braces for both branches in if statement if one needs it. (suggested
by Andrew Lunn)
* introduce compatible string marvell,armada-370-xp-gpio (suggest by
Rob Herring)
* fix mvebu_pwmreg_blink_on_duration -> mvebu_pwmreg_blink_off_duration
for period callculation in mvebu_pwm_get_state()
Patch 4/4 mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
* Drop flags from pwms for Mamba, as no longer used (suggested by
Andrew Lunn)
* Use again #pwm-cell = 2, the second cell is actually the period.
Changes v2->v3:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* drop pin from mvebu_pwn, can be infered (suggested by Thierry Reding)
* rename pwm to mvpwm so pwm can be used for pwm_device as in the API,
avoids some mental gymnastic.
* drop id from struct mvebu_gpio_chip, select blink counter in
mvebu_pwm_probe for all lines instead. We do not care about the
unused ones. I think a clear improvement in readability.
Makes coming up with a good comment simple as well.
* Switch to new atomic PWM API (suggested by Thierry Reding)
* rename use mvebu_gpioreg_blink_select to
mvebu_gpioreg_blink_counter_select.
* mark *_suspend() / *_resume() as __maybe_unused (suggested by Linus
Walleij)
* document #pwm-cells = 1 (suggested by Thierry Reding)
Patch 2/4 mvebu: xp: Add PWM properties to .dtsi files
* add missing reg-names / #pwm-cell properties to
armada-xp-mv78260.dtsi gpio1 node
* set pwm-cells = 1 (suggested by Thierry Reding)
All:
* always uppercase GPIO/PWM in prose (suggested by Thierry Reding)
Changes v1 -> v2:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* use BIT macro (suggested by Linus Walleij)
* move id from struct mvebu_pwm to struct mvebu_gpio_chip, implement
blink select as if else and comment on the chip id for code clarity
(to accommodate Linus Walleijs request for a code clarification /
comment. If you can word it better I'm all ears.)
* Move function comment mvebu_pwm_probe into the function itself.
---
Andrew Lunn (4):
gpio: mvebu: Add limited PWM support
ARM: dts: mvebu: Add PWM properties to .dtsi files
ARM: mvebu: Enable SENSORS_PWM_FAN in defconfig
ARM: dts: armada-xp: Use pwm-fan rather than gpio-fan
.../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
MAINTAINERS | 2 +
arch/arm/boot/dts/armada-370.dtsi | 19 +-
arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 8 +-
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 16 +-
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 19 +-
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 19 +-
arch/arm/configs/mvebu_v7_defconfig | 2 +
drivers/gpio/gpio-mvebu.c | 327 ++++++++++++++++++++-
9 files changed, 408 insertions(+), 36 deletions(-)
--
2.10.2
^ permalink raw reply
* Re: [PATCH v3 2/2] iio: dac: add support for stm32 DAC
From: Jonathan Cameron @ 2017-04-14 15:36 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o
In-Reply-To: <1491839390-2449-3-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 10/04/17 16:49, Fabrice Gasnier wrote:
> Add support for STMicroelectronics STM32 DAC. It's a 12-bit, voltage
> output digital-to-analog converter. It has two output channels, each
> with its own converter.
> It supports 8 bits or 12bits left/right aligned data format. Only
> 12bits right-aligned is used here. It has built-in noise or
> triangle waveform generator, and supports external triggers for
> conversions.
> Each channel can be used independently, with separate trigger, then
> separate IIO devices are used to handle this. Core driver is intended
> to share common resources such as clock, reset, reference voltage and
> registers.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
You did the constant attribute differently from how I would have done, but
it's a perfectly valid approach so that's fine with me. (I'll probably
regret that when it keeps getting copied form now on).
Looks good to me,
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> Changes in v3:
> - Fix powerdown, no need for 'enable', use 'three_state' as standard name
> for 'left floating' as per ABI documentation, instead of 'Hi-Z'.
>
> Changes in v2:
> - Define 'Hi-Z'/'enable' powerdown modes instead of using 'enable'
> attribute normally not used for DACs.
> - use 'reg' instead of 'st,dac-channel' property
> - Use macro to differentiate channels
> - Fix typos, remove leading '&' for functions
> - Add comments on single channel per device
> - Use devm_iio_device_register variant, removes need for .remove
> ---
> drivers/iio/dac/Kconfig | 15 ++
> drivers/iio/dac/Makefile | 2 +
> drivers/iio/dac/stm32-dac-core.c | 180 +++++++++++++++++++++
> drivers/iio/dac/stm32-dac-core.h | 51 ++++++
> drivers/iio/dac/stm32-dac.c | 334 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 582 insertions(+)
> create mode 100644 drivers/iio/dac/stm32-dac-core.c
> create mode 100644 drivers/iio/dac/stm32-dac-core.h
> create mode 100644 drivers/iio/dac/stm32-dac.c
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index d3084028..7198648 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -274,6 +274,21 @@ config MCP4922
> To compile this driver as a module, choose M here: the module
> will be called mcp4922.
>
> +config STM32_DAC
> + tristate "STMicroelectronics STM32 DAC"
> + depends on (ARCH_STM32 && OF) || COMPILE_TEST
> + depends on REGULATOR
> + select STM32_DAC_CORE
> + help
> + Say yes here to build support for STMicroelectronics STM32 Digital
> + to Analog Converter (DAC).
> +
> + This driver can also be built as a module. If so, the module
> + will be called stm32-dac.
> +
> +config STM32_DAC_CORE
> + tristate
> +
> config VF610_DAC
> tristate "Vybrid vf610 DAC driver"
> depends on OF
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index f01bf4a..afe8ae7 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -29,4 +29,6 @@ obj-$(CONFIG_MAX517) += max517.o
> obj-$(CONFIG_MAX5821) += max5821.o
> obj-$(CONFIG_MCP4725) += mcp4725.o
> obj-$(CONFIG_MCP4922) += mcp4922.o
> +obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
> +obj-$(CONFIG_STM32_DAC) += stm32-dac.o
> obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/stm32-dac-core.c b/drivers/iio/dac/stm32-dac-core.c
> new file mode 100644
> index 0000000..75e4878
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac-core.c
> @@ -0,0 +1,180 @@
> +/*
> + * This file is part of STM32 DAC driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
> + *
> + * License type: GPLv2
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +
> +#include "stm32-dac-core.h"
> +
> +/**
> + * struct stm32_dac_priv - stm32 DAC core private data
> + * @pclk: peripheral clock common for all DACs
> + * @rst: peripheral reset control
> + * @vref: regulator reference
> + * @common: Common data for all DAC instances
> + */
> +struct stm32_dac_priv {
> + struct clk *pclk;
> + struct reset_control *rst;
> + struct regulator *vref;
> + struct stm32_dac_common common;
> +};
> +
> +static struct stm32_dac_priv *to_stm32_dac_priv(struct stm32_dac_common *com)
> +{
> + return container_of(com, struct stm32_dac_priv, common);
> +}
> +
> +static const struct regmap_config stm32_dac_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 0x3fc,
> +};
> +
> +static int stm32_dac_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct stm32_dac_priv *priv;
> + struct regmap *regmap;
> + struct resource *res;
> + void __iomem *mmio;
> + int ret;
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mmio = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mmio))
> + return PTR_ERR(mmio);
> +
> + regmap = devm_regmap_init_mmio(dev, mmio, &stm32_dac_regmap_cfg);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> + priv->common.regmap = regmap;
> +
> + priv->vref = devm_regulator_get(dev, "vref");
> + if (IS_ERR(priv->vref)) {
> + ret = PTR_ERR(priv->vref);
> + dev_err(dev, "vref get failed, %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_enable(priv->vref);
> + if (ret < 0) {
> + dev_err(dev, "vref enable failed\n");
> + return ret;
> + }
> +
> + ret = regulator_get_voltage(priv->vref);
> + if (ret < 0) {
> + dev_err(dev, "vref get voltage failed, %d\n", ret);
> + goto err_vref;
> + }
> + priv->common.vref_mv = ret / 1000;
> + dev_dbg(dev, "vref+=%dmV\n", priv->common.vref_mv);
> +
> + priv->pclk = devm_clk_get(dev, "pclk");
> + if (IS_ERR(priv->pclk)) {
> + ret = PTR_ERR(priv->pclk);
> + dev_err(dev, "pclk get failed\n");
> + goto err_vref;
> + }
> +
> + ret = clk_prepare_enable(priv->pclk);
> + if (ret < 0) {
> + dev_err(dev, "pclk enable failed\n");
> + goto err_vref;
> + }
> +
> + priv->rst = devm_reset_control_get(dev, NULL);
> + if (!IS_ERR(priv->rst)) {
> + reset_control_assert(priv->rst);
> + udelay(2);
> + reset_control_deassert(priv->rst);
> + }
> +
> + /* When clock speed is higher than 80MHz, set HFSEL */
> + priv->common.hfsel = (clk_get_rate(priv->pclk) > 80000000UL);
> + ret = regmap_update_bits(regmap, STM32_DAC_CR, STM32H7_DAC_CR_HFSEL,
> + priv->common.hfsel ? STM32H7_DAC_CR_HFSEL : 0);
> + if (ret)
> + goto err_pclk;
> +
> + platform_set_drvdata(pdev, &priv->common);
> +
> + ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to populate DT children\n");
> + goto err_pclk;
> + }
> +
> + return 0;
> +
> +err_pclk:
> + clk_disable_unprepare(priv->pclk);
> +err_vref:
> + regulator_disable(priv->vref);
> +
> + return ret;
> +}
> +
> +static int stm32_dac_remove(struct platform_device *pdev)
> +{
> + struct stm32_dac_common *common = platform_get_drvdata(pdev);
> + struct stm32_dac_priv *priv = to_stm32_dac_priv(common);
> +
> + of_platform_depopulate(&pdev->dev);
> + clk_disable_unprepare(priv->pclk);
> + regulator_disable(priv->vref);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_dac_of_match[] = {
> + { .compatible = "st,stm32h7-dac-core", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dac_of_match);
> +
> +static struct platform_driver stm32_dac_driver = {
> + .probe = stm32_dac_probe,
> + .remove = stm32_dac_remove,
> + .driver = {
> + .name = "stm32-dac-core",
> + .of_match_table = stm32_dac_of_match,
> + },
> +};
> +module_platform_driver(stm32_dac_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 DAC core driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stm32-dac-core");
> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
> new file mode 100644
> index 0000000..daf0993
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac-core.h
> @@ -0,0 +1,51 @@
> +/*
> + * This file is part of STM32 DAC driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
> + *
> + * License type: GPLv2
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __STM32_DAC_CORE_H
> +#define __STM32_DAC_CORE_H
> +
> +#include <linux/regmap.h>
> +
> +/* STM32 DAC registers */
> +#define STM32_DAC_CR 0x00
> +#define STM32_DAC_DHR12R1 0x08
> +#define STM32_DAC_DHR12R2 0x14
> +#define STM32_DAC_DOR1 0x2C
> +#define STM32_DAC_DOR2 0x30
> +
> +/* STM32_DAC_CR bit fields */
> +#define STM32_DAC_CR_EN1 BIT(0)
> +#define STM32H7_DAC_CR_HFSEL BIT(15)
> +#define STM32_DAC_CR_EN2 BIT(16)
> +
> +/**
> + * struct stm32_dac_common - stm32 DAC driver common data (for all instances)
> + * @regmap: DAC registers shared via regmap
> + * @vref_mv: reference voltage (mv)
> + * @hfsel: high speed bus clock selected
> + */
> +struct stm32_dac_common {
> + struct regmap *regmap;
> + int vref_mv;
> + bool hfsel;
> +};
> +
> +#endif
> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
> new file mode 100644
> index 0000000..50f8ec0
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac.c
> @@ -0,0 +1,334 @@
> +/*
> + * This file is part of STM32 DAC driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Authors: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> + * Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> + *
> + * License type: GPLv2
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include "stm32-dac-core.h"
> +
> +#define STM32_DAC_CHANNEL_1 1
> +#define STM32_DAC_CHANNEL_2 2
> +#define STM32_DAC_IS_CHAN_1(ch) ((ch) & STM32_DAC_CHANNEL_1)
> +
> +/**
> + * struct stm32_dac - private data of DAC driver
> + * @common: reference to DAC common data
> + */
> +struct stm32_dac {
> + struct stm32_dac_common *common;
> +};
> +
> +static int stm32_dac_is_enabled(struct iio_dev *indio_dev, int channel)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 en, val;
> + int ret;
> +
> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
> + if (ret < 0)
> + return ret;
> + if (STM32_DAC_IS_CHAN_1(channel))
> + en = FIELD_GET(STM32_DAC_CR_EN1, val);
> + else
> + en = FIELD_GET(STM32_DAC_CR_EN2, val);
> +
> + return !!en;
> +}
> +
> +static int stm32_dac_set_enable_state(struct iio_dev *indio_dev, int ch,
> + bool enable)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 msk = STM32_DAC_IS_CHAN_1(ch) ? STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
> + u32 en = enable ? msk : 0;
> + int ret;
> +
> + ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, msk, en);
> + if (ret < 0) {
> + dev_err(&indio_dev->dev, "%s failed\n", en ?
> + "Enable" : "Disable");
> + return ret;
> + }
> +
> + /*
> + * When HFSEL is set, it is not allowed to write the DHRx register
> + * during 8 clock cycles after the ENx bit is set. It is not allowed
> + * to make software/hardware trigger during this period either.
> + */
> + if (en && dac->common->hfsel)
> + udelay(1);
> +
> + return 0;
> +}
> +
> +static int stm32_dac_get_value(struct stm32_dac *dac, int channel, int *val)
> +{
> + int ret;
> +
> + if (STM32_DAC_IS_CHAN_1(channel))
> + ret = regmap_read(dac->common->regmap, STM32_DAC_DOR1, val);
> + else
> + ret = regmap_read(dac->common->regmap, STM32_DAC_DOR2, val);
> +
> + return ret ? ret : IIO_VAL_INT;
> +}
> +
> +static int stm32_dac_set_value(struct stm32_dac *dac, int channel, int val)
> +{
> + int ret;
> +
> + if (STM32_DAC_IS_CHAN_1(channel))
> + ret = regmap_write(dac->common->regmap, STM32_DAC_DHR12R1, val);
> + else
> + ret = regmap_write(dac->common->regmap, STM32_DAC_DHR12R2, val);
> +
> + return ret;
> +}
> +
> +static int stm32_dac_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return stm32_dac_get_value(dac, chan->channel, val);
> + case IIO_CHAN_INFO_SCALE:
> + *val = dac->common->vref_mv;
> + *val2 = chan->scan_type.realbits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int stm32_dac_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return stm32_dac_set_value(dac, chan->channel, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned reg, unsigned writeval,
> + unsigned *readval)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + if (!readval)
> + return regmap_write(dac->common->regmap, reg, writeval);
> + else
> + return regmap_read(dac->common->regmap, reg, readval);
> +}
> +
> +static const struct iio_info stm32_dac_iio_info = {
> + .read_raw = stm32_dac_read_raw,
> + .write_raw = stm32_dac_write_raw,
> + .debugfs_reg_access = stm32_dac_debugfs_reg_access,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static const char * const stm32_dac_powerdown_modes[] = {
> + "three_state",
> +};
> +
> +static int stm32_dac_get_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + return 0;
> +}
> +
> +static int stm32_dac_set_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int type)
> +{
> + return 0;
> +}
Slightly novel usage of the enum code, but I suppose it does no harm.
I'd have gone with an IIO_CONST_ATTR but this does line up nicely
with other drivers so other than taking a few more lines, not much wrong
with it as an approach.
> +
> +static ssize_t stm32_dac_read_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + int ret = stm32_dac_is_enabled(indio_dev, chan->channel);
> +
> + if (ret < 0)
> + return ret;
> +
> + return sprintf(buf, "%d\n", ret ? 0 : 1);
> +}
> +
> +static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + bool powerdown;
> + int ret;
> +
> + ret = strtobool(buf, &powerdown);
> + if (ret)
> + return ret;
> +
> + ret = stm32_dac_set_enable_state(indio_dev, chan->channel, !powerdown);
> + if (ret)
> + return ret;
> +
> + return len;
> +}
> +
> +static const struct iio_enum stm32_dac_powerdown_mode_en = {
> + .items = stm32_dac_powerdown_modes,
> + .num_items = ARRAY_SIZE(stm32_dac_powerdown_modes),
> + .get = stm32_dac_get_powerdown_mode,
> + .set = stm32_dac_set_powerdown_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
> + {
> + .name = "powerdown",
> + .read = stm32_dac_read_powerdown,
> + .write = stm32_dac_write_powerdown,
> + .shared = IIO_SEPARATE,
> + },
> + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &stm32_dac_powerdown_mode_en),
> + IIO_ENUM_AVAILABLE("powerdown_mode", &stm32_dac_powerdown_mode_en),
> + {},
> +};
> +
> +#define STM32_DAC_CHANNEL(chan, name) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = chan, \
> + .info_mask_separate = \
> + BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + /* scan_index is always 0 as num_channels is 1 */ \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .datasheet_name = name, \
> + .ext_info = stm32_dac_ext_info \
> +}
> +
> +static const struct iio_chan_spec stm32_dac_channels[] = {
> + STM32_DAC_CHANNEL(STM32_DAC_CHANNEL_1, "out1"),
> + STM32_DAC_CHANNEL(STM32_DAC_CHANNEL_2, "out2"),
> +};
> +
> +static int stm32_dac_chan_of_init(struct iio_dev *indio_dev)
> +{
> + struct device_node *np = indio_dev->dev.of_node;
> + unsigned int i;
> + u32 channel;
> + int ret;
> +
> + ret = of_property_read_u32(np, "reg", &channel);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Failed to read reg property\n");
> + return ret;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(stm32_dac_channels); i++) {
> + if (stm32_dac_channels[i].channel == channel)
> + break;
> + }
> + if (i >= ARRAY_SIZE(stm32_dac_channels)) {
> + dev_err(&indio_dev->dev, "Invalid st,dac-channel\n");
> + return -EINVAL;
> + }
> +
> + indio_dev->channels = &stm32_dac_channels[i];
> + /*
> + * Expose only one channel here, as they can be used independently,
> + * with separate trigger. Then separate IIO devices are instantiated
> + * to manage this.
> + */
> + indio_dev->num_channels = 1;
> +
> + return 0;
> +};
> +
> +static int stm32_dac_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev;
> + struct stm32_dac *dac;
> + int ret;
> +
> + if (!np)
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*dac));
> + if (!indio_dev)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, indio_dev);
> +
> + dac = iio_priv(indio_dev);
> + dac->common = dev_get_drvdata(pdev->dev.parent);
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->info = &stm32_dac_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = stm32_dac_chan_of_init(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
> +
> +static const struct of_device_id stm32_dac_of_match[] = {
> + { .compatible = "st,stm32-dac", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dac_of_match);
> +
> +static struct platform_driver stm32_dac_driver = {
> + .probe = stm32_dac_probe,
> + .driver = {
> + .name = "stm32-dac",
> + .of_match_table = stm32_dac_of_match,
> + },
> +};
> +module_platform_driver(stm32_dac_driver);
> +
> +MODULE_ALIAS("platform:stm32-dac");
> +MODULE_AUTHOR("Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 DAC driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [PATCH 3/8] ARM: dts: imx7s: Adjust anatop-enable-bit for 'reg_1p0d'
From: Dong Aisheng @ 2017-04-14 15:32 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Shawn Guo, yurovsky-Re5JQEeQqe8AvxtiuMwx3w, Sascha Hauer,
Fabio Estevam, Rob Herring, Mark Rutland, Russell King,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170413133242.5068-4-andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thu, Apr 13, 2017 at 06:32:37AM -0700, Andrey Smirnov wrote:
> In PMU_REG_1P0Dn ENABLE_LINREG is bit 0. Bit 31 is called OVERRIDE and
> it serves the function of granting permission to GPC IP block to alter
> various bit-fields of the register. The reason why this property, that
> trickeld here from Freescale BSP, is set to 31 is because in the code
> it came from it is used in conjunction with a notifier handler for
> REGULATOR_EVENT_PRE_DO_ENABLE and REGULATOR_EVENT_PRE_DO_DISABLE
> events (not found in upstream kernel) that triggers GPC to start
> manipulating aforementioned other bitfields.
>
> Since:
> a) none of the aforementioned machinery is implemented by
> upstream
> b) using 'anatop-enable-bit' in that capacity is a bit of a
> semantic stretch
Yes, this does is a bit of semantic stretch.
FSL using is combined with regulator notify and that do bring a bit
of complexity.
I'm not sure if it's good to introduce another anatop-override-bit
to separate, but i'm a bit scare since there's already many....
>
> simplify the situation by setting the value of 'anatop-enable-bit' to
> point to ENABLE_LINREG (same as i.MX6).
>
> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> arch/arm/boot/dts/imx7s.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 22c9788..8fee299 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -516,7 +516,7 @@
> anatop-min-bit-val = <8>;
> anatop-min-voltage = <800000>;
> anatop-max-voltage = <1200000>;
> - anatop-enable-bit = <31>;
> + anatop-enable-bit = <0>;
The change of this line seems already exist in patch 1.
Regards
Dong Aisheng
--
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 6/8] ARM: dts: imx7d-sdb: Add GPIO expander node
From: Andrey Smirnov @ 2017-04-14 15:32 UTC (permalink / raw)
To: Shawn Guo
Cc: Andrey Yurovsky, Sascha Hauer, Fabio Estevam, Rob Herring,
Mark Rutland, Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel, linux-arm-kernel
In-Reply-To: <20170414034659.GH14915@dragon>
On Thu, Apr 13, 2017 at 8:47 PM, Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote:
>> Add node for U38, a 74LV595PW serial-in shift register that acts as a
>> GPIO expander on the board.
>>
>> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
>> index 5be01a1..e0ff276 100644
>> --- a/arch/arm/boot/dts/imx7d-sdb.dts
>> +++ b/arch/arm/boot/dts/imx7d-sdb.dts
>> @@ -52,6 +52,30 @@
>> reg = <0x80000000 0x80000000>;
>> };
>>
>> + spi4 {
>> + compatible = "spi-gpio";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_spi1>;
>> + status = "okay";
>
> The 'status' is not needed in this case.
>
Missed that, will fix in v2.
>> + gpio-sck = <&gpio1 13 0>;
>> + gpio-mosi = <&gpio1 9 0>;
>> + cs-gpios = <&gpio1 12 0>;
>> + num-chipselects = <1>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + gpio_spi: gpio_spi@0 {
>
> gpio-expander might be a better node name?
>
Yeah, I agree. I'll change it to extended_io: gpio-expander@0
("Extended IO" is how this part is called out on the schematic)
>> + compatible = "fairchild,74hc595";
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + reg = <0>;
>> + registers-number = <1>;
>> + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/
>> + registers-default = /bits/ 8 <0x74>;
>
> I do not see this property is documented or supported by kernel.
My bad, some downstream properties leakage. Will remove in v2.
Thanks,
Andrey Smirnov
--
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 5/5] iio: dac: stm32: add support for waveform generator
From: Jonathan Cameron @ 2017-04-14 15:28 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o
In-Reply-To: <779a0082-3760-75c4-2fd3-b8a5b70dbaf8-qxv4g6HH51o@public.gmane.org>
On 10/04/17 17:43, Fabrice Gasnier wrote:
> On 04/09/2017 11:34 AM, Jonathan Cameron wrote:
>> On 06/04/17 17:11, Fabrice Gasnier wrote:
>>> STM32 DAC has built-in noise or triangle waveform generator.
>>> - "wavetype" extended attribute selects noise or triangle.
>>> - "amplitude" extended attribute selects amplitude for waveform generator
>>>
>>> A DC offset can be added to waveform generator output. This can be done
>>> using out_voltage[1/2]_offset
>>>
>>> Waveform generator requires a trigger to be configured, to increment /
>>> decrement internal counter in case of triangle generator. Noise
>>> generator is a bit different, but also requires a trigger to generate
>>> samples.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>>
>> Various bits inline. Mostly I think the blockers on this will be
>> making sure the ABI defined is generic enough to handle the more crazy
>> DDS chips out there... (basically the ones doing frequency modulation).
>>
>> Jonathan
>>> ---
>>> Changes in v2:
>>> - use _offset parameter to add DC offset to waveform generator
>> Conceptually this offset is just the normal DAC output value (particularly
> yes
>> in the flat case) I guess we can paper over this by having the _raw
>> and this always have th same value, but it's a little inelegant.
>> Still people are going to expect _raw to control it when in DAC mode but
>> that makes limited sense in DDS mode.
>>
>> Mind you nothing stops us defining all DDS channels as the sum of whatever
>> the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
>> purely through documentation. Think of the DDS as a modulation on top
>> of the DAC...
>>
>>> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>>> out_voltage_wavetype_available, out_voltageY_amplitude,
>>> out_voltage_amplitude_available
>> Hmm. I'm thinking those amplitude values aren't nice and don't fit well
>> with the more general ABI.
>>
>> I suggested (but didn't really expand upon) having standard defined types
>> for each waveform then using scale to control the amplitude.
>
> Do you mean _scale attribute ?
Yes
>>
>> Is that something that might work here?
>
> I probably miss the point here...
>>
>> So say we have our triangle standard form having an amplitude of 1V Peak to
>> Peak. Then we can use scale to make it whatever we actually have in this
>> case? The docs for wave type will need to describe those standard forms
>> though.
> ... scale is fixed here, in line with _raw attribute. In 'amplitude'
> description for STM32 DAC here (e.g. from 1...4095), same scale is used.
> Can you elaborate ?
Good point - it is already effectively been applied to the _raw
value - I'd forgotten that.
Seems like abuse of the ABI to use calibscale, so we might need something
new here - wavescale maybe or ddsscale? Not sure.
>
>>
>> Hmm. Whether this is worth doing is unclear however as we'll still have
>> to describe the 'frequency' in terms of the clock ticks (here the triggers)
>
> Describing frequency may be an issue, not sure it makes senses in this
> case: 'clock ticks', e.g. triggers here may be timers, but also an EXTI
> (external...), or even software trig perhaps.
To describe the waveform in a remotely standard way we'll have to address
this to some degree. What's the slope?
>
>> So maybe amplitude is worth having. Again, looking for input from ADI lot
>> on this... There are some really fiddly cases to describe were we are doing
>> symbol encoding so have multiple waveforms that we are switching between based
>> on some external signal. Any ABI needs to encompass that sort of usage.
>> Parts like the AD9833 for example...
>>
>>> - Better explain trigger usage in case of waveform generator.
>>> ---
>>> Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 | 16 +++
>>> drivers/iio/dac/stm32-dac-core.h | 4 +
>>> drivers/iio/dac/stm32-dac.c | 158 +++++++++++++++++++++-
>>> 3 files changed, 177 insertions(+), 1 deletion(-)
>>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>> new file mode 100644
>>> index 0000000..8f1fa009
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> Fair enough to initially introduced these for this part only, but I'd very
>> much like to see us agree on these sufficiently to get them into the main
>> docs asap so we have something to work with for getting the DDS chips out
>> of staging...
>>> @@ -0,0 +1,16 @@
>>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
>>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
>>> +KernelVersion: 4.12
>>> +Contact: fabrice.gasnier-qxv4g6HH51o@public.gmane.org
>>> +Description:
>>> + List and/or select waveform generation provided by STM32 DAC:
>>> + - "flat": waveform generator disabled (default)
>>> + - "noise": select noise waveform
>>> + - "triangle": select triangle waveform
>>> +
>>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
>>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
>>> +KernelVersion: 4.12
>>> +Contact: fabrice.gasnier-qxv4g6HH51o@public.gmane.org
>>> +Description:
>>> + List and/or select amplitude used for waveform generator
>>> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
>>> index e51a468..0f02975 100644
>>> --- a/drivers/iio/dac/stm32-dac-core.h
>>> +++ b/drivers/iio/dac/stm32-dac-core.h
>>> @@ -37,8 +37,12 @@
>>> #define STM32H7_DAC_CR_TEN1 BIT(1)
>>> #define STM32H7_DAC_CR_TSEL1_SHIFT 2
>>> #define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
>>> +#define STM32_DAC_CR_WAVE1 GENMASK(7, 6)
>>> +#define STM32_DAC_CR_MAMP1 GENMASK(11, 8)
>>> #define STM32H7_DAC_CR_HFSEL BIT(15)
>>> #define STM32_DAC_CR_EN2 BIT(16)
>>> +#define STM32_DAC_CR_WAVE2 GENMASK(23, 22)
>>> +#define STM32_DAC_CR_MAMP2 GENMASK(27, 24)
>>>
>>> /* STM32_DAC_SWTRIGR bit fields */
>>> #define STM32_DAC_SWTRIGR_SWTRIG1 BIT(0)
>>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>>> index a7a078e..2ed75db 100644
>>> --- a/drivers/iio/dac/stm32-dac.c
>>> +++ b/drivers/iio/dac/stm32-dac.c
>>> @@ -42,10 +42,12 @@
>>> /**
>>> * struct stm32_dac - private data of DAC driver
>>> * @common: reference to DAC common data
>>> + * @wavetype: waveform generator
>>> * @swtrig: Using software trigger
>>> */
>>> struct stm32_dac {
>>> struct stm32_dac_common *common;
>>> + u32 wavetype;
>>> bool swtrig;
>>> };
>>>
>>> @@ -222,6 +224,29 @@ static int stm32_dac_set_value(struct stm32_dac *dac, int channel, int val)
>>> return ret;
>>> }
>>>
>>> +static int stm32_dac_get_offset(struct stm32_dac *dac, int channel, int *val)
>>> +{
>>> + int ret;
>>> +
>>> + /* Offset is only relevant in waveform generation mode. */
>>> + if (!dac->wavetype) {
>>> + *val = 0;
>>> + return IIO_VAL_INT;
>>> + }
>>> +
>>> + /*
>>> + * In waveform generation mode, DC offset in DHR is added to waveform
>>> + * generator output, then stored to DOR (data output register).
>>> + * Read offset from DHR.
>>> + */
>> Just thinking what fun we could have if we do the fifo based output to push
>> this that I was suggesting for the previous patch ;) triangles on top
>> of fun general waveforms..
>>
>>> + if (STM32_DAC_IS_CHAN_1(channel))
>>> + ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R1, val);
>>> + else
>>> + ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R2, val);
>>> +
>>> + return ret ? ret : IIO_VAL_INT;
>>> +}
>>> +
>>> static int stm32_dac_read_raw(struct iio_dev *indio_dev,
>>> struct iio_chan_spec const *chan,
>>> int *val, int *val2, long mask)
>>> @@ -231,6 +256,8 @@ static int stm32_dac_read_raw(struct iio_dev *indio_dev,
>>> switch (mask) {
>>> case IIO_CHAN_INFO_RAW:
>>> return stm32_dac_get_value(dac, chan->channel, val);
>>> + case IIO_CHAN_INFO_OFFSET:
>>> + return stm32_dac_get_offset(dac, chan->channel, val);
>>> case IIO_CHAN_INFO_SCALE:
>>> *val = dac->common->vref_mv;
>>> *val2 = chan->scan_type.realbits;
>>> @@ -247,8 +274,16 @@ static int stm32_dac_write_raw(struct iio_dev *indio_dev,
>>> struct stm32_dac *dac = iio_priv(indio_dev);
>>>
>>> switch (mask) {
>>> + case IIO_CHAN_INFO_OFFSET:
>>> + /* Offset only makes sense in waveform generation mode */
>>> + if (dac->wavetype)
>>> + return stm32_dac_set_value(dac, chan->channel, val);
>>> + return -EBUSY;
>> Yeah, I think I sent you down a blind alley here. If people agree, lets
>> just define DDS signals as always being the sum of _raw + the dds element.
>> Then it's easy.
> Ok, I can revert back to use _raw if this is fine ;-)
>
>>> case IIO_CHAN_INFO_RAW:
>>> - return stm32_dac_set_value(dac, chan->channel, val);
>>> + if (!dac->wavetype)
>>> + return stm32_dac_set_value(dac, chan->channel, val);
>>> + /* raw value is read only in waveform generation mode */
>>> + return -EBUSY;
>>> default:
>>> return -EINVAL;
>>> }
>>> @@ -334,6 +369,122 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
>>> .set = stm32_dac_set_powerdown_mode,
>>> };
>>>
>>> +/* waveform generator wave selection */
>>> +static const char * const stm32_dac_wavetype_desc[] = {
>>> + "flat",
>>> + "noise",
>>> + "triangle",
>>> +};
>>> +
>>> +static int stm32_dac_set_wavetype(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + unsigned int wavetype)
>>> +{
>>> + struct stm32_dac *dac = iio_priv(indio_dev);
>>> + u32 mask, val;
>>> + int ret;
>>> +
>>> + /*
>>> + * Waveform generator requires a trigger to be configured, to increment
>>> + * or decrement internal counter in case of triangle generator. Noise
>>> + * generator is a bit different, but also requires a trigger to
>>> + * generate samples.
>>> + */
>>> + if (wavetype && !indio_dev->trig)
>>> + dev_dbg(&indio_dev->dev, "Wavegen requires a trigger\n");
>>> +
>>> + if (STM32_DAC_IS_CHAN_1(chan->channel)) {
>>> + val = FIELD_PREP(STM32_DAC_CR_WAVE1, wavetype);
>>> + mask = STM32_DAC_CR_WAVE1;
>>> + } else {
>>> + val = FIELD_PREP(STM32_DAC_CR_WAVE2, wavetype);
>>> + mask = STM32_DAC_CR_WAVE2;
>>> + }
>>> +
>>> + ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, mask, val);
>>> + if (ret)
>>> + return ret;
>>> + dac->wavetype = wavetype;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_dac_get_wavetype(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan)
>>> +{
>>> + struct stm32_dac *dac = iio_priv(indio_dev);
>>> + u32 val;
>>> + int ret;
>>> +
>>> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (STM32_DAC_IS_CHAN_1(chan->channel))
>>> + return FIELD_GET(STM32_DAC_CR_WAVE1, val);
>>> + else
>>> + return FIELD_GET(STM32_DAC_CR_WAVE2, val);
>>> +}
>>> +
>>> +static const struct iio_enum stm32_dac_wavetype_enum = {
>>> + .items = stm32_dac_wavetype_desc,
>>> + .num_items = ARRAY_SIZE(stm32_dac_wavetype_desc),
>>> + .get = stm32_dac_get_wavetype,
>>> + .set = stm32_dac_set_wavetype,
>>> +};
>>> +
>>> +/*
>>> + * waveform generator mamp selection: mask/amplitude
>>> + * - noise: LFSR mask (linear feedback shift register, umasks bit 0, [1:0]...)
>> This needs breaking out into two attributes - for noise it isn't amplitude in
>> any conventional sense... Keep the result device specific for now. I'm not
>> even sure what type of pseudo random source that actually is...
> Do you suggest to create specific attribute for this ? This will end-up
> to write same register/bitfield as 'amplitude' for triangle generator.
I think it should definitely be a separate attribute. Otherwise the userspace
ABI will be really confusing as this definitely isn't amplitude in any normal
sense!
>
> Thanks & Regards,
> Fabrice
>
>> If anyone wants to figure it out it would be great to document it in a general
>> form!
>>
>>> + * - triangle: amplitude (equal to 1, 3, 5, 7... 4095)
>>> + */
>>> +static const char * const stm32_dac_amplitude_desc[] = {
>>> + "1", "3", "7", "15", "31", "63", "127", "255", "511", "1023", "2047",
>>> + "4095",
>>> +};
>>> +
>>> +static int stm32_dac_set_amplitude(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + unsigned int amplitude)
>>> +{
>>> + struct stm32_dac *dac = iio_priv(indio_dev);
>>> + u32 mask, val;
>>> +
>>> + if (STM32_DAC_IS_CHAN_1(chan->channel)) {
>>> + val = FIELD_PREP(STM32_DAC_CR_MAMP1, amplitude);
>>> + mask = STM32_DAC_CR_MAMP1;
>>> + } else {
>>> + val = FIELD_PREP(STM32_DAC_CR_MAMP2, amplitude);
>>> + mask = STM32_DAC_CR_MAMP2;
>>> + }
>>> +
>>> + return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, mask, val);
>>> +}
>>> +
>>> +static int stm32_dac_get_amplitude(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan)
>>> +{
>>> + struct stm32_dac *dac = iio_priv(indio_dev);
>>> + u32 val;
>>> + int ret;
>>> +
>>> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (STM32_DAC_IS_CHAN_1(chan->channel))
>>> + return FIELD_GET(STM32_DAC_CR_MAMP1, val);
>>> + else
>>> + return FIELD_GET(STM32_DAC_CR_MAMP2, val);
>>> +}
>>> +
>>> +static const struct iio_enum stm32_dac_amplitude_enum = {
>>> + .items = stm32_dac_amplitude_desc,
>>> + .num_items = ARRAY_SIZE(stm32_dac_amplitude_desc),
>>> + .get = stm32_dac_get_amplitude,
>>> + .set = stm32_dac_set_amplitude,
>>> +};
>>> +
>>> static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
>>> {
>>> .name = "powerdown",
>>> @@ -343,6 +494,10 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
>>> },
>>> IIO_ENUM("powerdown_mode", IIO_SEPARATE, &stm32_dac_powerdown_mode_en),
>>> IIO_ENUM_AVAILABLE("powerdown_mode", &stm32_dac_powerdown_mode_en),
>>> + IIO_ENUM("wavetype", IIO_SEPARATE, &stm32_dac_wavetype_enum),
>>> + IIO_ENUM_AVAILABLE("wavetype", &stm32_dac_wavetype_enum),
>>> + IIO_ENUM("amplitude", IIO_SEPARATE, &stm32_dac_amplitude_enum),
>>> + IIO_ENUM_AVAILABLE("amplitude", &stm32_dac_amplitude_enum),
>>> {},
>>> };
>>>
>>> @@ -352,6 +507,7 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
>>> .output = 1, \
>>> .channel = chan, \
>>> .info_mask_separate = \
>>> + BIT(IIO_CHAN_INFO_OFFSET) | \
>>> BIT(IIO_CHAN_INFO_RAW) | \
>>> BIT(IIO_CHAN_INFO_SCALE), \
>>> /* scan_index is always 0 as num_channels is 1 */ \
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox