[RFC PATCH 1/2] net: macb: Correct CAPS mask
USRIO and JUMBO CAPS have the same mask. Fix the same. Signed-off-by: Harini Katakam --- drivers/net/ethernet/cadence/macb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index 36893d8..b6fcf10 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -403,11 +403,11 @@ #define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII0x0004 #define MACB_CAPS_NO_GIGABIT_HALF 0x0008 #define MACB_CAPS_USRIO_DISABLED 0x0010 +#define MACB_CAPS_JUMBO0x0020 #define MACB_CAPS_FIFO_MODE0x1000 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x2000 #define MACB_CAPS_SG_DISABLED 0x4000 #define MACB_CAPS_MACB_IS_GEM 0x8000 -#define MACB_CAPS_JUMBO0x0010 /* Bit manipulation macros */ #define MACB_BIT(name) \ -- 2.7.4
[RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
This patch adds support for 64 bit addressing and BDs.
-> Enable 64 bit addressing in DMACFG register.
-> Set DMA mask when design config register shows support for 64 bit addr.
-> Add new BD words for higher address when 64 bit DMA support is present.
-> Add and update TBQPH and RBQPH for MSB of BD pointers.
-> Change extraction and updation of buffer addresses to use
64 bit address.
-> In gem_rx extract address in one place insted of two and use a
separate flag for RXUSED.
Signed-off-by: Harini Katakam
---
drivers/net/ethernet/cadence/macb.c | 62 ++---
drivers/net/ethernet/cadence/macb.h | 10 ++
2 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c
b/drivers/net/ethernet/cadence/macb.c
index 89c0cfa..6b797e3 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -541,6 +541,14 @@ static void macb_tx_unmap(struct macb *bp, struct
macb_tx_skb *tx_skb)
}
}
+static inline void macb_set_addr(struct macb_dma_desc *desc, dma_addr_t addr)
+{
+ desc->addr = (u32)addr;
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ desc->addrh = (u32)(addr >> 32);
+#endif
+}
+
static void macb_tx_error_task(struct work_struct *work)
{
struct macb_queue *queue = container_of(work, struct macb_queue,
@@ -621,14 +629,17 @@ static void macb_tx_error_task(struct work_struct *work)
/* Set end of TX queue */
desc = macb_tx_desc(queue, 0);
- desc->addr = 0;
+ macb_set_addr(desc, 0);
desc->ctrl = MACB_BIT(TX_USED);
/* Make descriptor updates visible to hardware */
wmb();
/* Reinitialize the TX desc queue */
- queue_writel(queue, TBQP, queue->tx_ring_dma);
+ queue_writel(queue, TBQP, (u32)(queue->tx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ queue_writel(queue, TBQPH, (u32)(queue->tx_ring_dma >> 32));
+#endif
/* Make TX ring reflect state of hardware */
queue->tx_head = 0;
queue->tx_tail = 0;
@@ -750,7 +761,7 @@ static void gem_rx_refill(struct macb *bp)
if (entry == RX_RING_SIZE - 1)
paddr |= MACB_BIT(RX_WRAP);
- bp->rx_ring[entry].addr = paddr;
+ macb_set_addr(&(bp->rx_ring[entry]), paddr);
bp->rx_ring[entry].ctrl = 0;
/* properly align Ethernet header */
@@ -798,7 +809,9 @@ static int gem_rx(struct macb *bp, int budget)
int count = 0;
while (count < budget) {
- u32 addr, ctrl;
+ u32 ctrl;
+ dma_addr_t addr;
+ bool rxused;
entry = macb_rx_ring_wrap(bp->rx_tail);
desc = &bp->rx_ring[entry];
@@ -806,10 +819,14 @@ static int gem_rx(struct macb *bp, int budget)
/* Make hw descriptor updates visible to CPU */
rmb();
- addr = desc->addr;
+ rxused = (desc->addr & MACB_BIT(RX_USED)) ? true : false;
+ addr = MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ addr |= ((u64)(desc->addrh) << 32);
+#endif
ctrl = desc->ctrl;
- if (!(addr & MACB_BIT(RX_USED)))
+ if (!rxused)
break;
bp->rx_tail++;
@@ -835,7 +852,6 @@ static int gem_rx(struct macb *bp, int budget)
netdev_vdbg(bp->dev, "gem_rx %u (len %u)\n", entry, len);
skb_put(skb, len);
- addr = MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, addr));
dma_unmap_single(&bp->pdev->dev, addr,
bp->rx_buffer_size, DMA_FROM_DEVICE);
@@ -1299,7 +1315,7 @@ static unsigned int macb_tx_map(struct macb *bp,
ctrl |= MACB_BIT(TX_WRAP);
/* Set TX buffer descriptor */
- desc->addr = tx_skb->mapping;
+ macb_set_addr(desc, tx_skb->mapping);
/* desc->addr must be visible to hardware before clearing
* 'TX_USED' bit in desc->ctrl.
*/
@@ -1422,6 +1438,9 @@ static void gem_free_rx_buffers(struct macb *bp)
desc = &bp->rx_ring[i];
addr = MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ addr |= ((u64)(desc->addrh) << 32);
+#endif
dma_unmap_single(&bp->pdev->dev, addr, bp->rx_buffer_size,
DMA_FROM_DEVICE);
dev_kfree_skb_any(skb);
@@ -1547,7 +1566,7 @@ static void gem_init_rings(
Re: [RFC PATCH 1/2] macb: Add 1588 support in Cadence GEM.
Hi, On Tue, Sep 6, 2016 at 9:18 PM, Richard Cochran wrote: > >> +#define GEM_TISUBN 0x01bc /* 1588 Timer Increment Sub-ns */ > > This regsiter does not exist. Looking at > >Zynq-7000 AP SoC Technical Reference Manual >UG585 (v1.10) February 23, 2015 > > starting on page 1273 we see: > > udp_csum_errors 0x01B0 32 ro0x UDP checksum error > timer_strobe_s 0x01C8 32 rw0x 1588 timer sync strobe seconds > timer_strobe_ns 0x01CC 32 mixed 0x 1588 timer sync strobe > nanoseconds > timer_s 0x01D0 32 rw0x 1588 timer seconds > timer_ns0x01D4 32 mixed 0x 1588 timer nanoseconds > timer_adjust0x01D8 32 mixed 0x 1588 timer adjust > timer_incr 0x01DC 32 mixed 0x 1588 timer increment > > There is no register at 0x1BC. > >> +#define GEM_TSH 0x01c0 /* 1588 Timer Seconds High */ > > This one doesn't exist either. What is going on here? I cant be sure of the version of Cadence GEM used in SAMA5D2 but these registers (sub ns increments alteast) only exist in the IP version used in Zynq Ultrascale+ MPSoC. >> + /* get GEM internal time */ >> + sech = gem_readl(bp, TSH); >> + secl = gem_readl(bp, TSL); > > Does reading TSH latch the time? The TRM is silent about that, and > most other designs latch on reading the LSB. No, it does not latch the time. When doing a read + adjust + write, this will mean there's room for some error. Although when writing, the write to MSB and LSB registers was made atomic. This bug fix came only in the most recent version of the IP, am afraid. Regards, Harini
Re: [RFC PATCH 1/2] net: macb: Correct CAPS mask
On Thu, Aug 4, 2016 at 7:37 PM, Nicolas Ferre wrote:
> Le 01/08/2016 à 09:20, Harini Katakam a écrit :
>> USRIO and JUMBO CAPS have the same mask.
>> Fix the same.
>>
>> Signed-off-by: Harini Katakam
>
> Hi,
> Indeed there's a bug...
>
>
>> ---
>> drivers/net/ethernet/cadence/macb.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.h
>> b/drivers/net/ethernet/cadence/macb.h
>> index 36893d8..b6fcf10 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -403,11 +403,11 @@
>> #define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII 0x0004
>> #define MACB_CAPS_NO_GIGABIT_HALF0x0008
>> #define MACB_CAPS_USRIO_DISABLED 0x0010
>> +#define MACB_CAPS_JUMBO 0x0020
>> #define MACB_CAPS_FIFO_MODE 0x1000
>> #define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x2000
>> #define MACB_CAPS_SG_DISABLED0x4000
>> #define MACB_CAPS_MACB_IS_GEM0x8000
>> -#define MACB_CAPS_JUMBO 0x0010
>
> Acked-by: Nicolas Ferre
>
> Can you please send it independently without the RFC tag in the subject
> and with the following tags in the body as well:
>
> Fixes: ce721a702197 ("net: ethernet: cadence-macb: Add disabled usrio caps")
> Cc: sta...@vger.kernel.org # v4.5+
>
Thanks Nicolas. I'll do that.
Regards,
Harini
>> /* Bit manipulation macros */
>> #define MACB_BIT(name) \
>>
>
> Thanks, bye,
> --
> Nicolas Ferre
[PATCH] net: macb: Correct CAPS mask
USRIO and JUMBO CAPS have the same mask.
Fix the same.
Fixes: ce721a702197 ("net: ethernet: cadence-macb: Add disabled usrio caps")
Cc: sta...@vger.kernel.org # v4.5+
Signed-off-by: Harini Katakam
Acked-by: Nicolas Ferre
---
drivers/net/ethernet/cadence/macb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb.h
b/drivers/net/ethernet/cadence/macb.h
index 36893d8..b6fcf10 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -403,11 +403,11 @@
#define MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII0x0004
#define MACB_CAPS_NO_GIGABIT_HALF 0x0008
#define MACB_CAPS_USRIO_DISABLED 0x0010
+#define MACB_CAPS_JUMBO0x0020
#define MACB_CAPS_FIFO_MODE0x1000
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x2000
#define MACB_CAPS_SG_DISABLED 0x4000
#define MACB_CAPS_MACB_IS_GEM 0x8000
-#define MACB_CAPS_JUMBO0x0010
/* Bit manipulation macros */
#define MACB_BIT(name) \
--
2.7.4
[PATCH v2] net: macb: Handle HRESP error
From: Harini Katakam
Handle HRESP error by doing a SW reset of RX and TX and
re-initializing the descriptors, RX and TX queue pointers.
Signed-off-by: Harini Katakam
Signed-off-by: Michal Simek
---
v2:
Rebased on top of latest net-next and reinitialized
all rx queues.
drivers/net/ethernet/cadence/macb.h | 3 ++
drivers/net/ethernet/cadence/macb_main.c | 59 +---
2 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h
b/drivers/net/ethernet/cadence/macb.h
index c50c5ec..8665982 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
#define MACB_EXT_DESC
@@ -1200,6 +1201,8 @@ struct macb {
struct ethtool_rx_fs_list rx_fs_list;
spinlock_t rx_fs_lock;
unsigned int max_tuples;
+
+ struct tasklet_struct hresp_err_tasklet;
};
#ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index 234667e..e84afcf 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1258,6 +1258,57 @@ static int macb_poll(struct napi_struct *napi, int
budget)
return work_done;
}
+static void macb_hresp_error_task(unsigned long data)
+{
+ struct macb *bp = (struct macb *)data;
+ struct net_device *dev = bp->dev;
+ struct macb_queue *queue = bp->queues;
+ unsigned int q;
+ u32 ctrl;
+
+ for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+ queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+ }
+ ctrl = macb_readl(bp, NCR);
+ ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
+ macb_writel(bp, NCR, ctrl);
+
+ netif_tx_stop_all_queues(dev);
+ netif_carrier_off(dev);
+
+ bp->macbgem_ops.mog_init_rings(bp);
+
+ /* Initialize TX and RX buffers */
+ for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+ queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+ queue_writel(queue, RBQPH,
+upper_32_bits(queue->rx_ring_dma));
+#endif
+ queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ if (bp->hw_dma_cap & HW_DMA_CAP_64B)
+ queue_writel(queue, TBQPH,
+upper_32_bits(queue->tx_ring_dma));
+#endif
+
+ /* Enable interrupts */
+ queue_writel(queue, IER,
+MACB_RX_INT_FLAGS |
+MACB_TX_INT_FLAGS |
+MACB_BIT(HRESP));
+ }
+
+ ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
+ macb_writel(bp, NCR, ctrl);
+
+ netif_carrier_on(dev);
+ netif_tx_start_all_queues(dev);
+}
+
static irqreturn_t macb_interrupt(int irq, void *dev_id)
{
struct macb_queue *queue = dev_id;
@@ -1347,10 +1398,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
}
if (status & MACB_BIT(HRESP)) {
- /* TODO: Reset the hardware, and maybe move the
-* netdev_err to a lower-priority context as well
-* (work queue?)
-*/
+ tasklet_schedule(&bp->hresp_err_tasklet);
netdev_err(dev, "DMA bus error: HRESP not OK\n");
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
@@ -3937,6 +3985,9 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_unregister_mdio;
}
+ tasklet_init(&bp->hresp_err_tasklet, macb_hresp_error_task,
+(unsigned long)bp);
+
phy_attached_info(phydev);
netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
--
2.7.4
This email and any attachments are intended for the sole use of the named
recipient(s) and contain(s) confidential information that may be proprietary,
privileged or copyrighted under applicable law. If you are not the intended
recipient, do not read, copy, or forward this email message or any attachments.
Delete this email message and any attachments immediately.
Re: [PATCH v2] net: macb: Handle HRESP error
Hi David, On Fri, Jan 26, 2018 at 9:25 PM, David Miller wrote: > From: Harini Katakam > Date: Fri, 26 Jan 2018 16:12:11 +0530 > >> From: Harini Katakam >> >> Handle HRESP error by doing a SW reset of RX and TX and >> re-initializing the descriptors, RX and TX queue pointers. >> >> Signed-off-by: Harini Katakam >> Signed-off-by: Michal Simek >> --- >> v2: >> Rebased on top of latest net-next and reinitialized >> all rx queues. > > Your patch was corrupted by your email client, it changed TAB characters > into sequences of SPACES amongst other things. We cannot integrate your > changes until you fix this. I'll fix this. > >> This email and any attachments are intended for the sole use of the named >> recipient(s) and contain(s) confidential information that may be >> proprietary, privileged or copyrighted under applicable law. If you are not >> the intended recipient, do not read, copy, or forward this email message or >> any attachments. Delete this email message and any attachments immediately. > > This is also completely inappropriate for a public development list and > you must eliminate this signature on future postings or we will have to > ignore them. Sorry, I usually remove this, missed it this time. Will resend. Regards, Harini
Re: [PATCH v3] net: macb: Handle HRESP error
Hi David,
On Sat, Jan 27, 2018 at 12:09 PM, wrote:
> From: Harini Katakam
>
> Handle HRESP error by doing a SW reset of RX and TX and
> re-initializing the descriptors, RX and TX queue pointers.
>
> Signed-off-by: Harini Katakam
> Signed-off-by: Michal Simek
> ---
> v3 and v2 changes:
> Fixed patch formatting errors
> Rebased on latest net-next and reinitialized
> multiple rx queues in error handling.
>
> drivers/net/ethernet/cadence/macb.h | 3 ++
> drivers/net/ethernet/cadence/macb_main.c | 59
> +---
> 2 files changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h
> b/drivers/net/ethernet/cadence/macb.h
> index c50c5ec..8665982 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
>
> #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) || defined(CONFIG_MACB_USE_HWSTAMP)
> #define MACB_EXT_DESC
> @@ -1200,6 +1201,8 @@ struct macb {
> struct ethtool_rx_fs_list rx_fs_list;
> spinlock_t rx_fs_lock;
> unsigned int max_tuples;
> +
> + struct tasklet_struct hresp_err_tasklet;
> };
>
> #ifdef CONFIG_MACB_USE_HWSTAMP
> diff --git a/drivers/net/ethernet/cadence/macb_main.c
> b/drivers/net/ethernet/cadence/macb_main.c
> index 234667e..e84afcf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1258,6 +1258,57 @@ static int macb_poll(struct napi_struct *napi, int
> budget)
> return work_done;
> }
>
> +static void macb_hresp_error_task(unsigned long data)
> +{
> + struct macb *bp = (struct macb *)data;
> + struct net_device *dev = bp->dev;
> + struct macb_queue *queue = bp->queues;
> + unsigned int q;
> + u32 ctrl;
> +
> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> + queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
> +MACB_TX_INT_FLAGS |
> +MACB_BIT(HRESP));
> + }
> + ctrl = macb_readl(bp, NCR);
> + ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE));
> + macb_writel(bp, NCR, ctrl);
> +
> + netif_tx_stop_all_queues(dev);
> + netif_carrier_off(dev);
> +
> + bp->macbgem_ops.mog_init_rings(bp);
> +
> + /* Initialize TX and RX buffers */
> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> + queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> + if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> + queue_writel(queue, RBQPH,
> +upper_32_bits(queue->rx_ring_dma));
> +#endif
> + queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> + if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> + queue_writel(queue, TBQPH,
> +upper_32_bits(queue->tx_ring_dma));
> +#endif
> +
> + /* Enable interrupts */
> + queue_writel(queue, IER,
> +MACB_RX_INT_FLAGS |
> +MACB_TX_INT_FLAGS |
> +MACB_BIT(HRESP));
> + }
> +
> + ctrl |= MACB_BIT(RE) | MACB_BIT(TE);
> + macb_writel(bp, NCR, ctrl);
> +
> + netif_carrier_on(dev);
> + netif_tx_start_all_queues(dev);
> +}
> +
> static irqreturn_t macb_interrupt(int irq, void *dev_id)
> {
> struct macb_queue *queue = dev_id;
> @@ -1347,10 +1398,7 @@ static irqreturn_t macb_interrupt(int irq, void
> *dev_id)
> }
>
> if (status & MACB_BIT(HRESP)) {
> - /* TODO: Reset the hardware, and maybe move the
> -* netdev_err to a lower-priority context as well
> -* (work queue?)
> -*/
> + tasklet_schedule(&bp->hresp_err_tasklet);
> netdev_err(dev, "DMA bus error: HRESP not OK\n");
>
> if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> @@ -3937,6 +3985,9 @@ static int macb_probe(struct platform_device *pdev)
> goto err_out_unregister_mdio;
> }
>
> + tasklet_init(&bp->hresp_err_tasklet, macb_hresp_error_task,
> +(unsigned long)bp);
> +
> phy_attached_info(phydev);
>
> netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
> --
> 2.7.4
>
Apologies for the spam.
This is the v3. Please do let me know in case you still see any corruption.
Regards,
Harini
[RFC PATCH] net: macb: Apply RXUBR workaround only to versions with errata
The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91 only. Subsequent versions do not need this
workaround. This workaround unecessarily reset RX whenever RX used
bit read is observed, which can be often under heavy traffic.Hence
introduce an errata field and a check to enable this workaround.
Signed-off-by: Harini Katakam
---
Note: Enabled the errata in zynq and at91 configs only.
Please advise if any other versions are affected by this errata.
drivers/net/ethernet/cadence/macb.h | 6 ++
drivers/net/ethernet/cadence/macb_main.c | 9 +++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h
b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..f6903d6 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -648,6 +648,9 @@
#define MACB_CAPS_SG_DISABLED 0x4000
#define MACB_CAPS_MACB_IS_GEM 0x8000
+/* Errata mask bits */
+#define MACB_ERRATA_RXLOCKUP 0x0001
+
/* LSO settings */
#define MACB_LSO_UFO_ENABLE0x01
#define MACB_LSO_TSO_ENABLE0x02
@@ -1085,6 +1088,7 @@ struct macb_config {
struct clk **rx_clk);
int (*init)(struct platform_device *pdev);
int jumbo_max_len;
+ u32 errata;
};
struct tsu_incr {
@@ -1214,6 +1218,8 @@ struct macb {
int rx_bd_rd_prefetch;
int tx_bd_rd_prefetch;
+
+ u32 errata;
};
#ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index 1d86b4d..f0bb8a4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1379,7 +1379,8 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
* the at91 manual, section 41.3.1 or the Zynq manual
* section 16.7.4 for details.
*/
- if (status & MACB_BIT(RXUBR)) {
+ if ((bp->errata & MACB_ERRATA_RXLOCKUP) &&
+ (status & MACB_BIT(RXUBR))) {
ctrl = macb_readl(bp, NCR);
macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
wmb();
@@ -3835,6 +3836,7 @@ static const struct macb_config at91sam9260_config = {
.caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
.clk_init = macb_clk_init,
.init = macb_init,
+ .errata = MACB_ERRATA_RXLOCKUP,
};
static const struct macb_config sama5d3macb_config = {
@@ -3900,6 +3902,7 @@ static const struct macb_config zynq_config = {
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
+ .errata = MACB_ERRATA_RXLOCKUP,
};
static const struct of_device_id macb_dt_ids[] = {
@@ -4005,8 +4008,10 @@ static int macb_probe(struct platform_device *pdev)
bp->hclk = hclk;
bp->tx_clk = tx_clk;
bp->rx_clk = rx_clk;
- if (macb_config)
+ if (macb_config) {
bp->jumbo_max_len = macb_config->jumbo_max_len;
+ bp->errata = macb_config->errata;
+ }
bp->wol = 0;
if (of_get_property(np, "magic-packet", NULL))
--
2.7.4
[PATCH v2 4/4] net: macb: Add support for suspend/resume with full power down
When macb device is suspended and system is powered down, the clocks
are removed and hence macb should be closed gracefully and restored
upon resume. This patch does the same by switching off the net device,
suspending phy and performing necessary cleanup of interrupts and BDs.
Upon resume, all these are reinitialized again.
Reset of macb device is done only when GEM is not a wake device.
Even when gem is a wake device, tx queues can be stopped and ptp device
can be closed (tsu clock will be disabled in pm_runtime_suspend) as
wake event detection has no dependency on this.
Signed-off-by: Kedareswara rao Appana
Signed-off-by: Harini Katakam
---
v2 changes:
Fixed parameter passed to phy calls.
drivers/net/ethernet/cadence/macb_main.c | 38 ++--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index 4b85ad7..dcb0194 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4249,16 +4249,33 @@ static int __maybe_unused macb_suspend(struct device
*dev)
{
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+ struct macb_queue *queue = bp->queues;
+ unsigned long flags;
+ unsigned int q;
+
+ if (!netif_running(netdev))
+ return 0;
- netif_carrier_off(netdev);
- netif_device_detach(netdev);
if (bp->wol & MACB_WOL_ENABLED) {
macb_writel(bp, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, MACB_BIT(MAG));
enable_irq_wake(bp->queues[0].irq);
+ netif_device_detach(netdev);
+ } else {
+ netif_device_detach(netdev);
+ for (q = 0, queue = bp->queues; q < bp->num_queues; ++q,
++queue)
+ napi_disable(&queue->napi);
+ phy_stop(netdev->phydev);
+ phy_suspend(netdev->phydev);
+ spin_lock_irqsave(&bp->lock, flags);
+ macb_reset_hw(bp);
+ spin_unlock_irqrestore(&bp->lock, flags);
}
+ netif_carrier_off(netdev);
+ if (bp->ptp_info)
+ bp->ptp_info->ptp_remove(netdev);
pm_runtime_force_suspend(dev);
return 0;
@@ -4268,6 +4285,11 @@ static int __maybe_unused macb_resume(struct device *dev)
{
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+ struct macb_queue *queue = bp->queues;
+ unsigned int q;
+
+ if (!netif_running(netdev))
+ return 0;
pm_runtime_force_resume(dev);
@@ -4275,9 +4297,21 @@ static int __maybe_unused macb_resume(struct device *dev)
macb_writel(bp, IDR, MACB_BIT(WOL));
macb_writel(bp, WOL, 0);
disable_irq_wake(bp->queues[0].irq);
+ } else {
+ macb_writel(bp, NCR, MACB_BIT(MPE));
+ for (q = 0, queue = bp->queues; q < bp->num_queues; ++q,
++queue)
+ napi_enable(&queue->napi);
+ phy_resume(netdev->phydev);
+ phy_init_hw(netdev->phydev);
+ phy_start(netdev->phydev);
}
+ bp->macbgem_ops.mog_init_rings(bp);
+ macb_init_hw(bp);
+ macb_set_rx_mode(netdev);
netif_device_attach(netdev);
+ if (bp->ptp_info)
+ bp->ptp_info->ptp_init(netdev);
return 0;
}
--
2.7.4
[PATCH v4 0/2] Cadence I2C driver fixes
This series implements workarounds for some bugs in Cadence I2C controller. - The I2C controller when configured in Master Mode generates invalid read transactions. When HOLD bit is set and HW timeout occurs, invalid read transactions are generated on the bus. This will affect repeated start conditions and large data transfer condtions where transfer_size_register has to be updated again. The transfer size register rolls over erroneously under these conditions. Note that this HW timeout cannot be disabled even if the interrupt is unused. Errata link: http://www.xilinx.com/support/answers/61664.html -The I2C controller when configured in Master Mode is the missing master completion interrupt. During repeated start condition, the driver has to set the HOLD bit for the set of transfer being done together and clear the HOLD bit just before the last transfer. But the controller does not indicate completion when a read/receive transfer is done if the HOLD bit is set. This affects all repeated start operation where a read/receive transfer is not the last transfer. Errata link: http://www.xilinx.com/support/answers/61665.html To address the above, - >252 byte transfers are done such that transfer_size never becomes zero. - timeout register value is increased (even though the driver doesn't use this). - A check is performed to see if there is any transfer following a read/receive transfer in the set of messages using repeated start. An error is returned in such cases because if we proceed, completion interrupt is never obtained and a SW timeout will occur. Harini Katakam (2): i2c: cadence: Handle > 252 byte transfers i2c: cadence: Check for errata condition involving master receive drivers/i2c/busses/i2c-cadence.c | 187 +++--- 1 file changed, 114 insertions(+), 73 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 2/2] i2c: cadence: Check for errata condition involving master receive
Cadence I2C controller has the following bugs:
- completion indication is not given to the driver at the end of
a read/receive transfer with HOLD bit set.
- Invalid read transaction are generated on the bus when HW timeout
condition occurs with HOLD bit set.
As a result of the above, if a set of messages to be transferred with
repeated start includes any transfer following a read transfer,
completion is never indicated and timeout occurs.
Hence a check is implemented to return -EOPNOTSUPP for such sequences.
Signed-off-by: Harini Katakam
Signed-off-by: Vishnu Motghare
---
v4:
Use single dev_warn and make message grep-able.
v3:
Add warning in case of unsupported transfer.
v2:
Dont defeteature repeated start. Just check for unsupported conditions in the
driver and return error.
---
drivers/i2c/busses/i2c-cadence.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 5f5d4fa..f9269a6 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -541,6 +541,20 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
* processed with a repeated start.
*/
if (num > 1) {
+ /*
+* This controller does not give completion interrupt after a
+* master receive transfer if HOLD bit is set (repeated start),
+* resulting in SW timeout. Hence, if a receive transfer is
+* followed by any other transfer, an error is returned
+* indicating that this sequence is not supported.
+*/
+ for (count = 0; count < num-1; count++) {
+ if (msgs[count].flags & I2C_M_RD)
+ dev_warn(adap->dev.parent, "No support for "
+"repeated start when receive is "
+"followed by a transfer\n");
+ return -EOPNOTSUPP;
+ }
id->bus_hold_flag = 1;
reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
reg |= CDNS_I2C_CR_HOLD;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/2] i2c: cadence: Handle > 252 byte transfers
The I2C controller sends a NACK to the slave when transfer size register
reaches zero, irrespective of the hold bit. So, in order to handle transfers
greater than 252 bytes, the transfer size register has to be maintained at a
value >= 1. This patch implements the same.
The interrupt status is cleared at the beginning of the isr instead of
the end, to avoid missing any interrupts.
Signed-off-by: Harini Katakam
---
v4:
No changes
v3:
Add comments
v2:
No changes
---
drivers/i2c/busses/i2c-cadence.c | 173 ++
1 file changed, 100 insertions(+), 73 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 63f3f03..5f5d4fa 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -126,6 +126,7 @@
* @suspended: Flag holding the device's PM status
* @send_count:Number of bytes still expected to send
* @recv_count:Number of bytes still expected to receive
+ * @curr_recv_count: Number of bytes to be received in current transfer
* @irq: IRQ number
* @input_clk: Input clock to I2C controller
* @i2c_clk: Maximum I2C clock speed
@@ -144,6 +145,7 @@ struct cdns_i2c {
u8 suspended;
unsigned int send_count;
unsigned int recv_count;
+ unsigned int curr_recv_count;
int irq;
unsigned long input_clk;
unsigned int i2c_clk;
@@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
*/
static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
{
- unsigned int isr_status, avail_bytes;
- unsigned int bytes_to_recv, bytes_to_send;
+ unsigned int isr_status, avail_bytes, updatetx;
+ unsigned int bytes_to_send;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
irqreturn_t status = IRQ_NONE;
isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
+ cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
/* Handling nack and arbitration lost interrupt */
if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
@@ -195,89 +198,112 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
status = IRQ_HANDLED;
}
- /* Handling Data interrupt */
- if ((isr_status & CDNS_I2C_IXR_DATA) &&
- (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
- /* Always read data interrupt threshold bytes */
- bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
- id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
- avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
-
- /*
-* if the tranfer size register value is zero, then
-* check for the remaining bytes and update the
-* transfer size register.
-*/
- if (!avail_bytes) {
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
- cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
- CDNS_I2C_XFER_SIZE_OFFSET);
- else
- cdns_i2c_writereg(id->recv_count,
- CDNS_I2C_XFER_SIZE_OFFSET);
- }
+ /*
+* Check if transfer size register needs to be updated again for a
+* large data receive operation.
+*/
+ updatetx = 0;
+ if (id->recv_count > id->curr_recv_count)
+ updatetx = 1;
+
+ /* When receiving, handle data interrupt and completion interrupt */
+ if (id->p_recv_buf &&
+ ((isr_status & CDNS_I2C_IXR_COMP) ||
+(isr_status & CDNS_I2C_IXR_DATA))) {
+ /* Read data if receive data valid is set */
+ while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
+ CDNS_I2C_SR_RXDV) {
+ /*
+* Clear hold bit that was set for FIFO control if
+* RX data left is less than FIFO depth, unless
+* repeated start is selected.
+*/
+ if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
+ !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
- /* Process the data received */
- while (bytes_to_recv--)
*(id->p_recv_buf)++ =
cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
+ id->recv_count--;
+ id->curr_recv_count--;
- if (!id->bus_hold_flag &&
- (id->recv_count
Re: [PATCH v4 2/2] i2c: cadence: Check for errata condition involving master receive
Hi Soren,
On Wed, Dec 10, 2014 at 11:15 PM, Sören Brinkmann
wrote:
> On Wed, 2014-12-10 at 05:14PM +0530, Harini Katakam wrote:
>> Cadence I2C controller has the following bugs:
>> - completion indication is not given to the driver at the end of
>> a read/receive transfer with HOLD bit set.
>> - Invalid read transaction are generated on the bus when HW timeout
>> condition occurs with HOLD bit set.
>>
>> As a result of the above, if a set of messages to be transferred with
>> repeated start includes any transfer following a read transfer,
>> completion is never indicated and timeout occurs.
>> Hence a check is implemented to return -EOPNOTSUPP for such sequences.
>>
>> Signed-off-by: Harini Katakam
>> Signed-off-by: Vishnu Motghare
>> ---
>>
>> v4:
>> Use single dev_warn and make message grep-able.
>>
>> v3:
>> Add warning in case of unsupported transfer.
>>
>> v2:
>> Dont defeteature repeated start. Just check for unsupported conditions in the
>> driver and return error.
>>
>> ---
>> drivers/i2c/busses/i2c-cadence.c | 14 ++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-cadence.c
>> b/drivers/i2c/busses/i2c-cadence.c
>> index 5f5d4fa..f9269a6 100644
>> --- a/drivers/i2c/busses/i2c-cadence.c
>> +++ b/drivers/i2c/busses/i2c-cadence.c
>> @@ -541,6 +541,20 @@ static int cdns_i2c_master_xfer(struct i2c_adapter
>> *adap, struct i2c_msg *msgs,
>>* processed with a repeated start.
>>*/
>> if (num > 1) {
>> + /*
>> + * This controller does not give completion interrupt after a
>> + * master receive transfer if HOLD bit is set (repeated start),
>> + * resulting in SW timeout. Hence, if a receive transfer is
>> + * followed by any other transfer, an error is returned
>> + * indicating that this sequence is not supported.
>> + */
>> + for (count = 0; count < num-1; count++) {
>> + if (msgs[count].flags & I2C_M_RD)
>> + dev_warn(adap->dev.parent, "No support for "
>> + "repeated start when receive is "
>> + "followed by a transfer\n");
>
> Still not grepable. There must be no line breaks in the string!
This message will be printed in one single line.
Do you want this string to be grepable in the driver? Why is that necessary?
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] i2c: cadence: Check for errata condition involving master receive
Hi,
On Fri, Dec 5, 2014 at 12:04 AM, Wolfram Sang wrote:
>
>> + /*
>> + * This controller does not give completion interrupt after a
>> + * master receive transfer if HOLD bit is set (repeated start),
>> + * resulting in SW timeout. Hence, if a receive transfer is
>> + * followed by any other transfer, an error is returned
>> + * indicating that this sequence is not supported.
>> + */
>> + for (count = 0; count < num-1; count++) {
>> + if (msgs[count].flags & I2C_M_RD)
>> + return -EOPNOTSUPP;
>> + }
>
> Yeah, a lot better. Probably it would be good to inform the user with a
> warning what went wrong?
>
Sure. I'll add that.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
Hi, On Fri, Dec 5, 2014 at 12:02 AM, Wolfram Sang wrote: > >> + /* >> + * If the device is sending data If there is further >> + * data to be sent. Calculate the available space >> + * in FIFO and fill the FIFO with that many bytes. >> + */ > > This comment looks broken. In general, I think there should be more > comments explaining why things have to be done this way. > There's some grammatical errors here. Let me correct it and add more comments. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] i2c: cadence: Handle > 252 byte transfers
Hi, On Fri, Dec 5, 2014 at 11:11 AM, rajeev kumar wrote: > On Wed, Dec 3, 2014 at 6:05 PM, Harini Katakam wrote: >> The I2C controller sends a NACK to the slave when transfer size register >> reaches zero, irrespective of the hold bit. So, in order to handle transfers >> greater than 252 bytes, the transfer size register has to be maintained at a >> value >= 1. This patch implements the same. > > Why 252 Bytes ? Is it word allign or what ? > It is the maximum transfer size that can be written that is a multiple of the data interrupt (this occurs when the fifo has 14 bytes). I will include an explanation in driver as well. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/2] Cadence I2C driver fixes
This series implements workarounds for some bugs in Cadence I2C controller. - The I2C controller when configured in Master Mode generates invalid read transactions. When HOLD bit is set and HW timeout occurs, invalid read transactions are generated on the bus. This will affect repeated start conditions and large data transfer condtions where transfer_size_register has to be updated again. The transfer size register rolls over erroneously under these conditions. Note that this HW timeout cannot be disabled even if the interrupt is unused. Errata link: http://www.xilinx.com/support/answers/61664.html -The I2C controller when configured in Master Mode is the missing master completion interrupt. During repeated start condition, the driver has to set the HOLD bit for the set of transfer being done together and clear the HOLD bit just before the last transfer. But the controller does not indicate completion when a read/receive transfer is done if the HOLD bit is set. This affects all repeated start operation where a read/receive transfer is not the last transfer. Errata link: http://www.xilinx.com/support/answers/61665.html To address the above, - >252 byte transfers are done such that transfer_size never becomes zero. - timeout register value is increased (even though the driver doesn't use this). - A check is performed to see if there is any transfer following a read/receive transfer in the set of messages using repeated start. An error is returned in such cases because if we proceed, completion interrupt is never obtained and a SW timeout will occur. Harini Katakam (2): i2c: cadence: Handle > 252 byte transfers i2c: cadence: Check for errata condition involving master receive drivers/i2c/busses/i2c-cadence.c | 188 +++--- 1 file changed, 115 insertions(+), 73 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/2] i2c: cadence: Check for errata condition involving master receive
Cadence I2C controller has the following bugs:
- completion indication is not given to the driver at the end of
a read/receive transfer with HOLD bit set.
- Invalid read transaction are generated on the bus when HW timeout
condition occurs with HOLD bit set.
As a result of the above, if a set of messages to be transferred with
repeated start includes any transfer following a read transfer,
completion is never indicated and timeout occurs.
Hence a check is implemented to return -EOPNOTSUPP for such sequences.
Signed-off-by: Harini Katakam
Signed-off-by: Vishnu Motghare
---
v3:
Add warning in case of unsupported transfer.
v2:
Dont defeteature repeated start. Just check for unsupported conditions in the
driver and return error.
---
drivers/i2c/busses/i2c-cadence.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 5f5d4fa..3ea3b7a 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -541,6 +541,21 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
* processed with a repeated start.
*/
if (num > 1) {
+ /*
+* This controller does not give completion interrupt after a
+* master receive transfer if HOLD bit is set (repeated start),
+* resulting in SW timeout. Hence, if a receive transfer is
+* followed by any other transfer, an error is returned
+* indicating that this sequence is not supported.
+*/
+ for (count = 0; count < num-1; count++) {
+ if (msgs[count].flags & I2C_M_RD)
+ dev_warn(adap->dev.parent,
+ "No support for repeated start when\t");
+ dev_warn(adap->dev.parent,
+ "receive is followed by a transfer\n");
+ return -EOPNOTSUPP;
+ }
id->bus_hold_flag = 1;
reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
reg |= CDNS_I2C_CR_HOLD;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] i2c: cadence: Handle > 252 byte transfers
The I2C controller sends a NACK to the slave when transfer size register
reaches zero, irrespective of the hold bit. So, in order to handle transfers
greater than 252 bytes, the transfer size register has to be maintained at a
value >= 1. This patch implements the same.
The interrupt status is cleared at the beginning of the isr instead of
the end, to avoid missing any interrupts.
Signed-off-by: Harini Katakam
---
v3:
Add comments
v2:
No changes
---
drivers/i2c/busses/i2c-cadence.c | 173 ++
1 file changed, 100 insertions(+), 73 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 63f3f03..5f5d4fa 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -126,6 +126,7 @@
* @suspended: Flag holding the device's PM status
* @send_count:Number of bytes still expected to send
* @recv_count:Number of bytes still expected to receive
+ * @curr_recv_count: Number of bytes to be received in current transfer
* @irq: IRQ number
* @input_clk: Input clock to I2C controller
* @i2c_clk: Maximum I2C clock speed
@@ -144,6 +145,7 @@ struct cdns_i2c {
u8 suspended;
unsigned int send_count;
unsigned int recv_count;
+ unsigned int curr_recv_count;
int irq;
unsigned long input_clk;
unsigned int i2c_clk;
@@ -180,14 +182,15 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
*/
static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
{
- unsigned int isr_status, avail_bytes;
- unsigned int bytes_to_recv, bytes_to_send;
+ unsigned int isr_status, avail_bytes, updatetx;
+ unsigned int bytes_to_send;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
irqreturn_t status = IRQ_NONE;
isr_status = cdns_i2c_readreg(CDNS_I2C_ISR_OFFSET);
+ cdns_i2c_writereg(isr_status, CDNS_I2C_ISR_OFFSET);
/* Handling nack and arbitration lost interrupt */
if (isr_status & (CDNS_I2C_IXR_NACK | CDNS_I2C_IXR_ARB_LOST)) {
@@ -195,89 +198,112 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
status = IRQ_HANDLED;
}
- /* Handling Data interrupt */
- if ((isr_status & CDNS_I2C_IXR_DATA) &&
- (id->recv_count >= CDNS_I2C_DATA_INTR_DEPTH)) {
- /* Always read data interrupt threshold bytes */
- bytes_to_recv = CDNS_I2C_DATA_INTR_DEPTH;
- id->recv_count -= CDNS_I2C_DATA_INTR_DEPTH;
- avail_bytes = cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET);
-
- /*
-* if the tranfer size register value is zero, then
-* check for the remaining bytes and update the
-* transfer size register.
-*/
- if (!avail_bytes) {
- if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
- cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
- CDNS_I2C_XFER_SIZE_OFFSET);
- else
- cdns_i2c_writereg(id->recv_count,
- CDNS_I2C_XFER_SIZE_OFFSET);
- }
+ /*
+* Check if transfer size register needs to be updated again for a
+* large data receive operation.
+*/
+ updatetx = 0;
+ if (id->recv_count > id->curr_recv_count)
+ updatetx = 1;
+
+ /* When receiving, handle data interrupt and completion interrupt */
+ if (id->p_recv_buf &&
+ ((isr_status & CDNS_I2C_IXR_COMP) ||
+(isr_status & CDNS_I2C_IXR_DATA))) {
+ /* Read data if receive data valid is set */
+ while (cdns_i2c_readreg(CDNS_I2C_SR_OFFSET) &
+ CDNS_I2C_SR_RXDV) {
+ /*
+* Clear hold bit that was set for FIFO control if
+* RX data left is less than FIFO depth, unless
+* repeated start is selected.
+*/
+ if ((id->recv_count < CDNS_I2C_FIFO_DEPTH) &&
+ !id->bus_hold_flag)
+ cdns_i2c_clear_bus_hold(id);
- /* Process the data received */
- while (bytes_to_recv--)
*(id->p_recv_buf)++ =
cdns_i2c_readreg(CDNS_I2C_DATA_OFFSET);
+ id->recv_count--;
+ id->curr_recv_count--;
- if (!id->bus_hold_flag &&
- (id->recv_count
Re: [PATCH v5 2/2] i2c: cadence: Check for errata condition involving master receive
Hi,
On Tue, Jan 13, 2015 at 5:05 PM, Wolfram Sang wrote:
>> @@ -541,6 +541,18 @@ static int cdns_i2c_master_xfer(struct i2c_adapter
>> *adap, struct i2c_msg *msgs,
>>* processed with a repeated start.
>>*/
>> if (num > 1) {
>> + /*
>> + * This controller does not give completion interrupt after a
>> + * master receive transfer if HOLD bit is set (repeated start),
>> + * resulting in SW timeout. Hence, if a receive transfer is
>> + * followed by any other transfer, an error is returned
>> + * indicating that this sequence is not supported.
>> + */
>> + for (count = 0; count < num-1; count++) {
>> + if (msgs[count].flags & I2C_M_RD)
>> + dev_warn(adap->dev.parent, "No support for
>> repeated start when receive is followed by a transfer\n");
>> + return -EOPNOTSUPP;
>> + }
>> id->bus_hold_flag = 1;
>> reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
>> reg |= CDNS_I2C_CR_HOLD;
>
> Have you ever tested this code? There is a huge bug in it :(
I'm sorry. I'll send another patch with the braces for the if statement.
I seem to have tested with only the return statement and no dev_warn.
>
> Also, in the comment, use "message" instead of "transfer". One transfer
> consists of multiple messages.
>
> Maybe the warning can be simplified, too: "can't do repeated start after
> read messages".
>
I'll do that.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/2] i2c: cadence: Handle > 252 byte transfers
Hi,
On Tue, Jan 13, 2015 at 4:58 PM, Wolfram Sang wrote:
> On Fri, Dec 12, 2014 at 09:48:26AM +0530, Harini Katakam wrote:
>> The I2C controller sends a NACK to the slave when transfer size register
>> reaches zero, irrespective of the hold bit. So, in order to handle transfers
>> greater than 252 bytes, the transfer size register has to be maintained at a
>> value >= 1. This patch implements the same.
>> The interrupt status is cleared at the beginning of the isr instead of
>> the end, to avoid missing any interrupts.
>>
>> Signed-off-by: Harini Katakam
>
> Applied to for-next, thanks!
>
>> @@ -333,10 +359,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>>* receive if it is less than transfer size and transfer size if
>>* it is more. Enable the interrupts.
>>*/
>> - if (id->recv_count > CDNS_I2C_TRANSFER_SIZE)
>> + if (id->recv_count > CDNS_I2C_TRANSFER_SIZE) {
>> cdns_i2c_writereg(CDNS_I2C_TRANSFER_SIZE,
>> CDNS_I2C_XFER_SIZE_OFFSET);
>> - else
>> + id->curr_recv_count = CDNS_I2C_TRANSFER_SIZE;
>> + } else
>> cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
>
> else branch must have braces, too! I fixed that.
Thanks!
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v6] i2c: cadence: Check for errata condition involving master receive
Cadence I2C controller has the following bugs:
- completion indication is not given to the driver at the end of
a read/receive transfer with HOLD bit set.
- Invalid read transaction are generated on the bus when HW timeout
condition occurs with HOLD bit set.
As a result of the above, if a set of messages to be transferred with
repeated start includes any message following a read message,
completion is never indicated and timeout occurs.
Hence a check is implemented to return -EOPNOTSUPP for such sequences.
Signed-off-by: Harini Katakam
Signed-off-by: Vishnu Motghare
---
v6:
- Correct if condition - add braces to include both statements.
- Modify comments and warning message.
v5:
Make warning grepable in driver.
v4:
Use single dev_warn and make message grep-able.
v3:
Add warning in case of unsupported transfer.
v2:
Dont defeteature repeated start. Just check for unsupported conditions in the
driver and return error.
---
drivers/i2c/busses/i2c-cadence.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 5f5d4fa..c7be4fb 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -541,6 +541,19 @@ static int cdns_i2c_master_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs,
* processed with a repeated start.
*/
if (num > 1) {
+ /*
+* This controller does not give completion interrupt after a
+* master receive message if HOLD bit is set (repeated start),
+* resulting in SW timeout. Hence, if a receive message is
+* followed by any other message, an error is returned
+* indicating that this sequence is not supported.
+*/
+ for (count = 0; count < num-1; count++) {
+ if (msgs[count].flags & I2C_M_RD) {
+ dev_warn(adap->dev.parent, "Can't do repeated
start after a receive message\n");
+ return -EOPNOTSUPP;
+ }
+ }
id->bus_hold_flag = 1;
reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
reg |= CDNS_I2C_CR_HOLD;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 0/2] Cadence I2C driver fixes
Hi, Any further comments on this? Regards, Harini On Fri, Dec 12, 2014 at 9:48 AM, Harini Katakam wrote: > This series implements workarounds for some bugs in Cadence I2C controller. > > - The I2C controller when configured in Master Mode generates invalid read > transactions. > When HOLD bit is set and HW timeout occurs, invalid read transactions are > generated on the bus. This will affect repeated start conditions and large > data transfer condtions where transfer_size_register has to be updated again. > The transfer size register rolls over erroneously under these conditions. > Note that this HW timeout cannot be disabled even if the interrupt is unused. > Errata link: http://www.xilinx.com/support/answers/61664.html > > -The I2C controller when configured in Master Mode is the missing master > completion interrupt. > During repeated start condition, the driver has to set the HOLD bit for > the set of transfer being done together and clear the HOLD bit just before > the last transfer. But the controller does not indicate completion when > a read/receive transfer is done if the HOLD bit is set. This affects > all repeated start operation where a read/receive transfer is not > the last transfer. > Errata link: http://www.xilinx.com/support/answers/61665.html > > To address the above, > - >252 byte transfers are done such that transfer_size never becomes zero. > - timeout register value is increased (even though the driver doesn't use > this). > - A check is performed to see if there is any transfer following a > read/receive transfer in the set of messages using repeated start. > An error is returned in such cases because if we proceed, completion interrupt > is never obtained and a SW timeout will occur. > > Harini Katakam (2): > i2c: cadence: Handle > 252 byte transfers > i2c: cadence: Check for errata condition involving master receive > > drivers/i2c/busses/i2c-cadence.c | 185 > +++--- > 1 file changed, 112 insertions(+), 73 deletions(-) > > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: cadence: Handling Slave monitor mode
Hi Wolfram, On Fri, Mar 27, 2015 at 9:07 PM, Wolfram Sang wrote: > On Tue, Mar 17, 2015 at 09:48:15PM +0530, Nava kishore Manne wrote: >> In slave monitor mode, the I2C interface is set up as a master and >> continues to attempt a transfer to a particular slave until the >> slave device responds with an ACK. >> >> Added this feature for zero length transfers enable the controller >> for slave monitor interrupt and get the status. Disable the slave >> monitor mode feature upon successful handling. >> >> Signed-off-by: Nava kishore Manne >> Acked-by: Harini Katakam > > I am not sure this is going to work. > > How often is this access tried when there is no device? It should be > tried only once. Once slave monitor is enabled in the host controller, it retries till there's an ACK or slave monitor is disabled. The SW wont be involved and need not be interrupted in this gap. The way this feature works in the controller though, I'm afraid there's no way to distinguish between no slave/ busy slave. The out is the software timeout in that case. Please suggest how to use this feature better. > > Those 0byte messages can be read or write, so transferring one bit, one > could say. I assume this one can only read? (which is not a show-stopper > but the cases should be handled) > Yes, only read is used. Thanks for the review. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller
Hi Mark,
On Thu, May 28, 2015 at 9:11 PM, Punnaiah Choudary Kalluri
wrote:
> Hi Mark,
>
>> -Original Message-
>> From: Mark Brown [mailto:broo...@kernel.org]
>> Sent: Thursday, May 28, 2015 8:34 PM
>> To: Harini Katakam
>> Cc: Ranjit Abhimanyu Waghmode; Rob Herring; Pawel Moll; Mark Rutland;
>> ijc+devicet...@hellion.org.uk; Kumar Gala; Michal Simek; Soren Brinkmann;
>> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>> ker...@vger.kernel.org; linux-spi; Punnaiah Choudary Kalluri;
>> ran27...@gmail.com
>> Subject: Re: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC
>> GQSPI controller
>>
>> On Fri, May 22, 2015 at 08:43:54PM +0530, Harini Katakam wrote:
>> > On Fri, May 22, 2015 at 5:28 PM, Mark Brown wrote:
>> > > On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote:
>>
>> > > Why is there a default case here? That's going to men we try to handle
>> > > any random chip select that gets passed in as pointing to this lower
>> > > device which doesn't seem right. The fact that this is trying to handle
>> > > mirroring of the chip select to two devices is also raising alarm bells
>> > > here...
>>
>> > This SPI controller has two CS lines and two data bus.
>> > Two devices can be connected to these and either the upper or the
>> > lower or both (Explained below) can be selected.
>>
>> > When two flash devices are used, one of the HW configurations in
>> > which they can be connected is called "parallel" mode where they
>>
>> I know what wiring chip selects in parallel is but that's not the
>> question - the question is about the handling of the default case.
>
> Yes, we should not handle default case here. We will change this.
>
Yes, that's right. I was just elaborating the mirroring part.
>>
>> > >> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool
>> is_high)
>> > >> +{
>>
>> > >> + if (is_high) {
>> > >> + /* Manually start the generic FIFO command */
>> > >> + zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST,
>> > >> + zynqmp_gqspi_read(xqspi,
>> > >> GQSPI_CONFIG_OFST) |
>> > >> + GQSPI_CFG_START_GEN_FIFO_MASK);
>>
>> > > No, this is broken - setting the chip select should set the chip select,
>> > > it shouldn't have any impact on transfers. Transfers should be started
>> > > in the transfer operations.
>>
>> > This is the only way to assert the CS. It doesn't start transferring any
>> > data.
>>
>> OK, then you can't implement a separate set_cs() operation and shouldn't
>> be trying to do so. This will break in multiple ways when the framework
>> tries to use the operations separately. You probably need to implement
>> a single flat transfer() operation.
>
> As we said it will not start any data transfer. In order to set chip select
> (chip select only) we need to
> 1. Frame a control word with following parameters like the chip select that
> we would like to set and hold time
> 2. Update the control word to fifo
>
> To have a performance advantage (may be not trivial) we are executing this
> fifo entry along with the first
> Data transfer entry in transfer function so that we can avoid waiting for
> fifo empty in set_cs function.
>
> We will ensure CS assert by waiting till the fifo empty in set_cs function
> and justify the what the
> Function supposed do.
>
Yes, if we wait for cs assert to be executed (which is just the time
controller takes to fetch this command
and execute), this set_cs will be independent as expected. The
framework can use it anytime.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Request for suggestion on net device re-naming/re-ordering based on DT alias
On Thu, Feb 28, 2019 at 12:10 AM Stephen Hemminger wrote: > > On Wed, 27 Feb 2019 17:24:03 +0530 > Harini Katakam wrote: > > > Device naming is a hard problem, and there is no perfect solution. > > Device tree should be providing hints to userspace policy for naming, not > trying to do it in the kernel. > See: > https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ Thanks Stephen - I'll try this. Regards, Harini
Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
Hi, On Wed, Feb 27, 2019 at 2:35 PM Harini Katakam wrote: > > Hi Andrew, Paul, > > On Wed, Feb 27, 2019 at 2:15 PM Michal Simek wrote: > > > > On 21. 02. 19 12:03, Michal Simek wrote: > > > On 21. 02. 19 11:24, Paul Kocialkowski wrote: > > >> Hi, > > >> > > >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote: > > >>> Hi, > > >>> > > >>> On 19. 02. 19 18:25, Andrew Lunn wrote: > > >> Understood. I think we need to start a discussion about how the general > > >> design of this driver can be improved. > > >> > > >> In particular, I wonder if it could work better to make this driver a > > >> PHY driver that just redirects all its ops to the actual PHY driver, > > >> except for read_status where it should also add some code. > > Thanks, I'm looking into this option and also a way to expose the correct > interface mode setting as you mentioned below. I'll get back before the > end of the week. Please do let me know if you have any further suggestions. > This IP does not have a PHY identifier or status register that can be accessed from the phy framework. We could discuss with our design team to add these in the future. But that would take sometime and this version should be still be supported. Also, if this IP has a PHY driver, then two phy drivers would have to be probed which are connected in a serial manner and I believe I'll have to update the framework to support that. Could you please let me know if you have any inputs on this? OR since this is just a bridge IP, is it acceptable to address the error cases? -> Module loading/unloading -> Spinlocks for protection -> Correct phy mode information to the driver. -> Any other concerns I could do a respin of this patch after addressing Andrew's comments: https://patchwork.kernel.org/patch/9290231/ Regards, Harini
[PATCH v3 4/4] net: macb: Add support for suspend/resume with full power down
When macb device is suspended and system is powered down, the clocks
are removed and hence macb should be closed gracefully and restored
upon resume. This patch does the same by switching off the net device,
suspending phy and performing necessary cleanup of interrupts and BDs.
Upon resume, all these are reinitialized again.
Reset of macb device is done only when GEM is not a wake device.
Even when gem is a wake device, tx queues can be stopped and ptp device
can be closed (tsu clock will be disabled in pm_runtime_suspend) as
wake event detection has no dependency on this.
Signed-off-by: Kedareswara rao Appana
Signed-off-by: Harini Katakam
---
v3:
Fix >80 char lines
v2 changes:
Fixed parameter passed to phy calls.
drivers/net/ethernet/cadence/macb_main.c | 40 ++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index 5173c02..ad099fd 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4283,16 +4283,34 @@ static int __maybe_unused macb_suspend(struct device
*dev)
{
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+ struct macb_queue *queue = bp->queues;
+ unsigned long flags;
+ unsigned int q;
+
+ if (!netif_running(netdev))
+ return 0;
- netif_carrier_off(netdev);
- netif_device_detach(netdev);
if (bp->wol & MACB_WOL_ENABLED) {
macb_writel(bp, IER, MACB_BIT(WOL));
macb_writel(bp, WOL, MACB_BIT(MAG));
enable_irq_wake(bp->queues[0].irq);
+ netif_device_detach(netdev);
+ } else {
+ netif_device_detach(netdev);
+ for (q = 0, queue = bp->queues; q < bp->num_queues;
+++q, ++queue)
+ napi_disable(&queue->napi);
+ phy_stop(netdev->phydev);
+ phy_suspend(netdev->phydev);
+ spin_lock_irqsave(&bp->lock, flags);
+ macb_reset_hw(bp);
+ spin_unlock_irqrestore(&bp->lock, flags);
}
+ netif_carrier_off(netdev);
+ if (bp->ptp_info)
+ bp->ptp_info->ptp_remove(netdev);
pm_runtime_force_suspend(dev);
return 0;
@@ -4302,6 +4320,11 @@ static int __maybe_unused macb_resume(struct device *dev)
{
struct net_device *netdev = dev_get_drvdata(dev);
struct macb *bp = netdev_priv(netdev);
+ struct macb_queue *queue = bp->queues;
+ unsigned int q;
+
+ if (!netif_running(netdev))
+ return 0;
pm_runtime_force_resume(dev);
@@ -4309,9 +4332,22 @@ static int __maybe_unused macb_resume(struct device *dev)
macb_writel(bp, IDR, MACB_BIT(WOL));
macb_writel(bp, WOL, 0);
disable_irq_wake(bp->queues[0].irq);
+ } else {
+ macb_writel(bp, NCR, MACB_BIT(MPE));
+ for (q = 0, queue = bp->queues; q < bp->num_queues;
+++q, ++queue)
+ napi_enable(&queue->napi);
+ phy_resume(netdev->phydev);
+ phy_init_hw(netdev->phydev);
+ phy_start(netdev->phydev);
}
+ bp->macbgem_ops.mog_init_rings(bp);
+ macb_init_hw(bp);
+ macb_set_rx_mode(netdev);
netif_device_attach(netdev);
+ if (bp->ptp_info)
+ bp->ptp_info->ptp_init(netdev);
return 0;
}
--
2.7.4
[PATCH v3 2/4] net: macb: Support clock management for tsu_clk
TSU clock needs to be enabled/disabled as per support in devicetree
and it should also be controlled during suspend/resume (WOL has no
dependency on this clock).
Signed-off-by: Harini Katakam
---
v3 and v2:
No changes
drivers/net/ethernet/cadence/macb.h | 3 ++-
drivers/net/ethernet/cadence/macb_main.c | 30 +-
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h
b/drivers/net/ethernet/cadence/macb.h
index f51040f..acc66a7 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1085,7 +1085,7 @@ struct macb_config {
unsigned intdma_burst_length;
int (*clk_init)(struct platform_device *pdev, struct clk **pclk,
struct clk **hclk, struct clk **tx_clk,
- struct clk **rx_clk);
+ struct clk **rx_clk, struct clk **tsu_clk);
int (*init)(struct platform_device *pdev);
int jumbo_max_len;
};
@@ -1165,6 +1165,7 @@ struct macb {
struct clk *hclk;
struct clk *tx_clk;
struct clk *rx_clk;
+ struct clk *tsu_clk;
struct net_device *dev;
union {
struct macb_stats macb;
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index a19ed2b..be10ee4 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3325,7 +3325,7 @@ static void macb_probe_queues(void __iomem *mem,
static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
struct clk **hclk, struct clk **tx_clk,
-struct clk **rx_clk)
+struct clk **rx_clk, struct clk **tsu_clk)
{
struct macb_platform_data *pdata;
int err;
@@ -3359,6 +3359,10 @@ static int macb_clk_init(struct platform_device *pdev,
struct clk **pclk,
if (IS_ERR(*rx_clk))
*rx_clk = NULL;
+ *tsu_clk = devm_clk_get(&pdev->dev, "tsu_clk");
+ if (IS_ERR(*tsu_clk))
+ *tsu_clk = NULL;
+
err = clk_prepare_enable(*pclk);
if (err) {
dev_err(&pdev->dev, "failed to enable pclk (%u)\n", err);
@@ -3383,8 +3387,17 @@ static int macb_clk_init(struct platform_device *pdev,
struct clk **pclk,
goto err_disable_txclk;
}
+ err = clk_prepare_enable(*tsu_clk);
+ if (err) {
+ dev_err(&pdev->dev, "failed to enable tsu_clk (%u)\n", err);
+ goto err_disable_rxclk;
+ }
+
return 0;
+err_disable_rxclk:
+ clk_disable_unprepare(*rx_clk);
+
err_disable_txclk:
clk_disable_unprepare(*tx_clk);
@@ -3835,13 +3848,14 @@ static const struct net_device_ops at91ether_netdev_ops
= {
static int at91ether_clk_init(struct platform_device *pdev, struct clk **pclk,
struct clk **hclk, struct clk **tx_clk,
- struct clk **rx_clk)
+ struct clk **rx_clk, struct clk **tsu_clk)
{
int err;
*hclk = NULL;
*tx_clk = NULL;
*rx_clk = NULL;
+ *tsu_clk = NULL;
*pclk = devm_clk_get(&pdev->dev, "ether_clk");
if (IS_ERR(*pclk))
@@ -3992,11 +4006,12 @@ static int macb_probe(struct platform_device *pdev)
{
const struct macb_config *macb_config = &default_gem_config;
int (*clk_init)(struct platform_device *, struct clk **,
- struct clk **, struct clk **, struct clk **)
- = macb_config->clk_init;
+ struct clk **, struct clk **, struct clk **,
+ struct clk **) = macb_config->clk_init;
int (*init)(struct platform_device *) = macb_config->init;
struct device_node *np = pdev->dev.of_node;
struct clk *pclk, *hclk = NULL, *tx_clk = NULL, *rx_clk = NULL;
+ struct clk *tsu_clk = NULL;
unsigned int queue_mask, num_queues;
struct macb_platform_data *pdata;
bool native_io;
@@ -4024,7 +4039,7 @@ static int macb_probe(struct platform_device *pdev)
}
}
- err = clk_init(pdev, &pclk, &hclk, &tx_clk, &rx_clk);
+ err = clk_init(pdev, &pclk, &hclk, &tx_clk, &rx_clk, &tsu_clk);
if (err)
return err;
@@ -4061,6 +4076,7 @@ static int macb_probe(struct platform_device *pdev)
bp->hclk = hclk;
bp->tx_clk = tx_clk;
bp->rx_clk = rx_clk;
+ bp->tsu_clk = tsu_clk;
if (macb_config)
bp->jumbo_max_len = macb_config->jumbo_max_len;
@@ -4180,6 +4196,7 @@ static int macb_p
[PATCH v3 1/4] net: macb: Check MDIO state before read/write and use timeouts
Replace the while loop in MDIO read/write functions with a timeout.
In addition, add a check for MDIO bus busy before initiating a new
operation as well to make sure there is no ongoing MDIO operation.
Signed-off-by: Shubhrajyoti Datta
Signed-off-by: Sai Pavan Boddu
Signed-off-by: Harini Katakam
Reviewed-by: Andrew Lunn
---
v3 changes:
Used MACB_BIT for IDLE field
v2 changes:
Use readx_poll_timeout
Changes form RFC:
Cleaned up timeout implementation and moved it to a helper.
drivers/net/ethernet/cadence/macb.h | 2 ++
drivers/net/ethernet/cadence/macb_main.c | 33 ++--
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h
b/drivers/net/ethernet/cadence/macb.h
index 9bbaad9..f51040f 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -715,6 +715,8 @@
__v; \
})
+#define MACB_READ_NSR(bp) macb_readl(bp, NSR)
+
/* struct macb_dma_desc - Hardware DMA descriptor
* @addr: DMA address of data buffer
* @ctrl: Control and status bits
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index f2915f2..a19ed2b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include "macb.h"
#define MACB_RX_BUFFER_SIZE128
@@ -79,6 +80,8 @@
*/
#define MACB_HALT_TIMEOUT 1230
+#define MACB_MDIO_TIMEOUT 100 /* in usecs */
+
/* DMA buffer descriptor might be different size
* depends on hardware configuration:
*
@@ -318,10 +321,23 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
}
+static int macb_mdio_wait_for_idle(struct macb *bp)
+{
+ u32 val;
+
+ return readx_poll_timeout(MACB_READ_NSR, bp, val, val & MACB_BIT(IDLE),
+ 1, MACB_MDIO_TIMEOUT);
+}
+
static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
{
struct macb *bp = bus->priv;
int value;
+ int err;
+
+ err = macb_mdio_wait_for_idle(bp);
+ if (err < 0)
+ return err;
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
| MACB_BF(RW, MACB_MAN_READ)
@@ -329,9 +345,9 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id,
int regnum)
| MACB_BF(REGA, regnum)
| MACB_BF(CODE, MACB_MAN_CODE)));
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
+ err = macb_mdio_wait_for_idle(bp);
+ if (err < 0)
+ return err;
value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
@@ -342,6 +358,11 @@ static int macb_mdio_write(struct mii_bus *bus, int
mii_id, int regnum,
u16 value)
{
struct macb *bp = bus->priv;
+ int err;
+
+ err = macb_mdio_wait_for_idle(bp);
+ if (err < 0)
+ return err;
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
| MACB_BF(RW, MACB_MAN_WRITE)
@@ -350,9 +371,9 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id,
int regnum,
| MACB_BF(CODE, MACB_MAN_CODE)
| MACB_BF(DATA, value)));
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
+ err = macb_mdio_wait_for_idle(bp);
+ if (err < 0)
+ return err;
return 0;
}
--
2.7.4
[PATCH v2 0/4] Macb power management support for ZynqMP
This series adds support for macb suspend/resume with system power down. In relation to the above, this series also updates mdio_read/write function for PM and adds tsu clock management. Harini Katakam (4): net: macb: Check MDIO state before read/write and use timeouts net: macb: Support clock management for tsu_clk net: macb: Add pm runtime support net: macb: Add support for suspend/resume with full power down drivers/net/ethernet/cadence/macb.h | 5 +- drivers/net/ethernet/cadence/macb_main.c | 215 ++- 2 files changed, 188 insertions(+), 32 deletions(-) -- 2.7.4
[PATCH v3 3/4] net: macb: Add pm runtime support
Add runtime pm functions and move clock handling there.
Add runtime PM calls to mdio functions to allow for active mdio bus.
Signed-off-by: Shubhrajyoti Datta
Signed-off-by: Harini Katakam
---
v3 changes:
Fix exit path using goto
v2 changes:
Allow for mdio bus to be active
Changes from RFC:
Updated pm get sync/put sync calls.
Removed unecessary clk up in mdio helpers.
drivers/net/ethernet/cadence/macb_main.c | 148 +++
1 file changed, 112 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c
b/drivers/net/ethernet/cadence/macb_main.c
index be10ee4..5173c02 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "macb.h"
#define MACB_RX_BUFFER_SIZE128
@@ -80,6 +81,8 @@
*/
#define MACB_HALT_TIMEOUT 1230
+#define MACB_PM_TIMEOUT 100 /* ms */
+
#define MACB_MDIO_TIMEOUT 100 /* in usecs */
/* DMA buffer descriptor might be different size
@@ -332,12 +335,15 @@ static int macb_mdio_wait_for_idle(struct macb *bp)
static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
{
struct macb *bp = bus->priv;
- int value;
- int err;
+ int status;
- err = macb_mdio_wait_for_idle(bp);
- if (err < 0)
- return err;
+ status = pm_runtime_get_sync(&bp->pdev->dev);
+ if (status < 0)
+ goto mdio_pm_exit;
+
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_read_exit;
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
| MACB_BF(RW, MACB_MAN_READ)
@@ -345,24 +351,32 @@ static int macb_mdio_read(struct mii_bus *bus, int
mii_id, int regnum)
| MACB_BF(REGA, regnum)
| MACB_BF(CODE, MACB_MAN_CODE)));
- err = macb_mdio_wait_for_idle(bp);
- if (err < 0)
- return err;
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_read_exit;
- value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
+ status = MACB_BFEXT(DATA, macb_readl(bp, MAN));
- return value;
+mdio_read_exit:
+ pm_runtime_mark_last_busy(&bp->pdev->dev);
+ pm_runtime_put_autosuspend(&bp->pdev->dev);
+mdio_pm_exit:
+ return status;
}
static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
u16 value)
{
struct macb *bp = bus->priv;
- int err;
+ int status;
- err = macb_mdio_wait_for_idle(bp);
- if (err < 0)
- return err;
+ status = pm_runtime_get_sync(&bp->pdev->dev);
+ if (status < 0)
+ goto mdio_pm_exit;
+
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_write_exit;
macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
| MACB_BF(RW, MACB_MAN_WRITE)
@@ -371,11 +385,15 @@ static int macb_mdio_write(struct mii_bus *bus, int
mii_id, int regnum,
| MACB_BF(CODE, MACB_MAN_CODE)
| MACB_BF(DATA, value)));
- err = macb_mdio_wait_for_idle(bp);
- if (err < 0)
- return err;
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_write_exit;
- return 0;
+mdio_write_exit:
+ pm_runtime_mark_last_busy(&bp->pdev->dev);
+ pm_runtime_put_autosuspend(&bp->pdev->dev);
+mdio_pm_exit:
+ return status;
}
/**
@@ -2418,12 +2436,18 @@ static int macb_open(struct net_device *dev)
netdev_dbg(bp->dev, "open\n");
+ err = pm_runtime_get_sync(&bp->pdev->dev);
+ if (err < 0)
+ goto pm_exit;
+
/* carrier starts down */
netif_carrier_off(dev);
/* if the phy is not yet register, retry later*/
- if (!dev->phydev)
- return -EAGAIN;
+ if (!dev->phydev) {
+ err = -EAGAIN;
+ goto pm_exit;
+ }
/* RX buffers initialization */
macb_init_rx_buffer_size(bp, bufsz);
@@ -2432,7 +2456,7 @@ static int macb_open(struct net_device *dev)
if (err) {
netdev_err(dev, "Unable to allocate DMA memory (error %d)\n",
err);
- return err;
+ goto pm_exit;
}
bp->macbgem_ops.mog_init_rings(bp);
@@ -2449,6 +2473,11 @@ static int macb_open(struct net_device *dev)
if (bp->ptp_info)
bp->ptp_info->ptp_init(dev);
+pm_exit:
+ if (err) {
+ pm_runtime_put_sync(&bp->pdev->dev);
+ return err;
+
Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
Hi Andrew, Paul, On Wed, Feb 27, 2019 at 2:15 PM Michal Simek wrote: > > On 21. 02. 19 12:03, Michal Simek wrote: > > On 21. 02. 19 11:24, Paul Kocialkowski wrote: > >> Hi, > >> > >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote: > >>> Hi, > >>> > >>> On 19. 02. 19 18:25, Andrew Lunn wrote: > > Thanks for the suggestion! So I had a closer look at that driver to try > > and see what could go wrong and it looks like I found a few things > > there. > > Hi Paul > > Yes, this driver has issues. If i remember correctly, it got merged > while i was on vacation. I pointed out a few issues, but the authors > never responded. Feel free to fix it up. > >>> Sorry for this - I've synced up with the author and got the comments from the time this driver was upstreamed. I'll try to address those and Paul's suggestions going forward. > >>> Will be good to know who was that person. > >>> > >>> I can't do much this week with this because responsible person for this > >>> driver is out of office this week. That's why please give us some time > >>> to get back to this. > >> > >> Understood. I think we need to start a discussion about how the general > >> design of this driver can be improved. > >> > >> In particular, I wonder if it could work better to make this driver a > >> PHY driver that just redirects all its ops to the actual PHY driver, > >> except for read_status where it should also add some code. Thanks, I'm looking into this option and also a way to expose the correct interface mode setting as you mentioned below. I'll get back before the end of the week. Please do let me know if you have any further suggestions. Regards, Harini > > > > I didn't take a look at Linux driver but it should work in a way that it > > checks description (more below) and then wait for attached phy to do its > > work and on the way back just setup this bridge based on that. > > > >> Maybe we could also manage to expose a RGMII PHY mode to the actual PHY > >> this way. Currently, the PHY mode has to be set to GMII for the MAC to > >> be configured correctly, but the PHY also gets this information while > >> it should be told that RGMII is in use. This doesn't seem to play a big > >> role in PHY configuration though, but it's still inadequate. > >> > >> What do you think?
Request for suggestion on net device re-naming/re-ordering based on DT alias
Hi,
We've had some users requesting control over net device name order
when multiple ethernet devices are present on a system. I've tried a
few solutions to this and looked it up on forums. But I apologize if
I have missed something.
I know that the current system allocates eth as per probe order
but that is obviously not stably controlled by user (tried DT
re-ordering and defer probe). One solution is to use DT alias names
to write to (net_device)->name as follows:
Devicetree:
aliases {
ethernet0 = &mac1;
ethernet1 = &mac0;
};
Driver probe:
+ /* Read ethernet DT alias id and assign to right device name*/
+ id = of_alias_get_id(np, "ethernet");
+ if (id < 0) {
+ dev_warn(&pdev->dev, "failed to get alias id (%u)\n", id);
+ return id;
+ }
+ snprintf(dev->name, sizeof(dev->name), "eth%d", id);
+
These three drivers seem to have something similar for mdio/phy bus IDs:
drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
of_alias_get_id(dn, "eth");
drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
of_alias_get_id(np, "ethernet");
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
plat->bus_id = of_alias_get_id(np, "ethernet");
Drawback: This approach will break if alias is not provided for one
of the interfaces on board. Not to mention, there could be systems
with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
If one of the drivers does not have an alias read mechanism, it is
possible to have clashing ID assignments. Is there any way this
solution can be changed to be stable/acceptable?
One other alternative I've tried is netdev kernel bootargs but this
device name was not being picked by the kernel:
https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
netdev= , , eth0
netdev=, , eth1
Could you please suggest any alternatives?
Thanks!
Regards,
Harini
[PATCH] SPI: Add driver for Cadence SPI controller
Add driver for Cadence SPI controller. This is used in Xilinx Zynq.
Signed-off-by: Harini Katakam
---
.../devicetree/bindings/spi/spi-cadence.txt| 25 +
drivers/spi/Kconfig|7 +
drivers/spi/Makefile |1 +
drivers/spi/spi-cadence.c | 804
4 files changed, 837 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-cadence.txt
create mode 100644 drivers/spi/spi-cadence.c
diff --git a/Documentation/devicetree/bindings/spi/spi-cadence.txt
b/Documentation/devicetree/bindings/spi/spi-cadence.txt
new file mode 100644
index 000..d25bc2d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-cadence.txt
@@ -0,0 +1,25 @@
+Cadence SPI controller Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "cdns,spi-r1p6".
+- reg : Physical base address and size of SPI registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- clock-names : List of input clock names - "ref_clk", "pclk"
+ (See clock bindings for details).
+- clocks : Clock phandles (see clock bindings for details).
+- num-chip-select : Number of chip selects used.
+
+Example:
+
+ spi@e0007000 {
+ clock-names = "ref_clk", "pclk";
+ clocks = <&clkc 26>, <&clkc 35>;
+ compatible = "cdns,spi-r1p6";
+ interrupt-parent = <&intc>;
+ interrupts = <0 49 4>;
+ num-chip-select = /bits/ 16 <4>;
+ reg = <0xe0007000 0x1000>;
+ } ;
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 581ee2a..aeae44a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -148,6 +148,13 @@ config SPI_BUTTERFLY
inexpensive battery powered microcontroller evaluation board.
This same cable can be used to flash new firmware.
+config SPI_CADENCE
+ tristate "Cadence SPI controller"
+ depends on SPI_MASTER
+ help
+ This selects the Cadence SPI controller master driver
+ used by Xilinx Zynq.
+
config SPI_CLPS711X
tristate "CLPS711X host SPI controller"
depends on ARCH_CLPS711X
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 95af48d..1be2ed7 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_SPI_BFIN_V3) += spi-bfin-v3.o
obj-$(CONFIG_SPI_BFIN_SPORT) += spi-bfin-sport.o
obj-$(CONFIG_SPI_BITBANG) += spi-bitbang.o
obj-$(CONFIG_SPI_BUTTERFLY)+= spi-butterfly.o
+obj-$(CONFIG_SPI_CADENCE) += spi-cadence.o
obj-$(CONFIG_SPI_CLPS711X) += spi-clps711x.o
obj-$(CONFIG_SPI_COLDFIRE_QSPI)+= spi-coldfire-qspi.o
obj-$(CONFIG_SPI_DAVINCI) += spi-davinci.o
diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
new file mode 100644
index 000..7344411
--- /dev/null
+++ b/drivers/spi/spi-cadence.c
@@ -0,0 +1,804 @@
+/*
+ * Cadence SPI controller driver (master mode only)
+ *
+ * Copyright (C) 2008 - 2014 Xilinx, Inc.
+ *
+ * based on Blackfin On-Chip SPI Driver (spi_bfin5xx.c)
+ *
+ * This program is free software; you can redistribute it and/or modify it
under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Name of this driver */
+#define CDNS_SPI_NAME "cdns-spi"
+
+/* Register offset definitions */
+#define CDNS_SPI_CR_OFFSET 0x00 /* Configuration Register, RW */
+#define CDNS_SPI_ISR_OFFSET0x04 /* Interrupt Status Register, RO */
+#define CDNS_SPI_IER_OFFSET0x08 /* Interrupt Enable Register, WO */
+#define CDNS_SPI_IDR_OFFSET0x0c /* Interrupt Disable Register, WO */
+#define CDNS_SPI_IMR_OFFSET0x10 /* Interrupt Enabled Mask Register, RO */
+#define CDNS_SPI_ER_OFFSET 0x14 /* Enable/Disable Register, RW */
+#define CDNS_SPI_DR_OFFSET 0x18 /* Delay Register, RW */
+#define CDNS_SPI_TXD_OFFSET0x1C /* Data Transmit Register, WO */
+#define CDNS_SPI_RXD_OFFSET0x20 /* Data Receive Register, RO */
+#define CDNS_SPI_SICR_OFFSET 0x24 /* Slave Idle Count Register, RW */
+#define CDNS_SPI_THLD_OFFSET 0x28 /* Transmit FIFO Watermark Register,RW */
+
+/*
+ * SPI Configuration Register bit Masks
+ *
+ * This register contains various control bits that affect the operation
+ * of the SPI controller
+ */
RE: [PATCH] SPI: Add driver for Cadence SPI controller
Hi Rob/Mark,
> -Original Message-
> From: Michal Simek [mailto:mon...@monstr.eu]
> Sent: Monday, March 17, 2014 6:53 PM
> To: Rob Herring
> Cc: Harini Katakam; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell;
> Kumar Gala; Rob Landley; Mark Brown; Grant Likely;
> devicet...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> Hi Rob,
>
> On 03/17/2014 01:47 PM, Rob Herring wrote:
> > On Mon, Mar 17, 2014 at 7:05 AM, Harini Katakam
> wrote:
> >> Add driver for Cadence SPI controller. This is used in Xilinx Zynq.
> >>
> >> Signed-off-by: Harini Katakam
> >> ---
> >> .../devicetree/bindings/spi/spi-cadence.txt| 25 +
> >
> > We prefer binding docs in separate patch.
>
> I have heard several times that also for binding review you need driver to
> look if this binding make sense that's why Harini sent this together.
> It means 2 emails instead of one.
> But if you wish to have this in two files no problem to split it but then I
> believe both should be copied to DT mailing list.
>
I can split this.
>
> >> drivers/spi/Kconfig|7 +
> >> drivers/spi/Makefile |1 +
> >> drivers/spi/spi-cadence.c | 804
> >>
> >> 4 files changed, 837 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/spi/spi-cadence.txt
> >> create mode 100644 drivers/spi/spi-cadence.c
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >> b/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >> new file mode 100644
> >> index 000..d25bc2d
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >> @@ -0,0 +1,25 @@
> >> +Cadence SPI controller Device Tree Bindings
> >> +---
> >> +
> >> +Required properties:
> >> +- compatible : Should be "cdns,spi-r1p6".
> >
> > One problem with using IP vendor info in the compatible string is an
> > IP block typically has a variety of configuration options so the
> > actual implementations may be very different. I would recommend adding
> > a Xilinx compatible string as well even if you don't use it now.
>
> It means xlnx,zynq-spi-r1p6 should be fine.
I can add this string. "r1p6" is specific to the cadence IP used.
Is that OK?
>
> >
> >> +- reg : Physical base address and size of SPI registers
> >> map.
> >> +- interrupts : Property with a value describing the interrupt
> >> + number.
> >> +- interrupt-parent : Must be core interrupt controller
> >> +- clock-names : List of input clock names - "ref_clk", "pclk"
> >> + (See clock bindings for details).
> >> +- clocks : Clock phandles (see clock bindings for details).
> >> +- num-chip-select : Number of chip selects used.
> >
> > See Documentation/devicetree/bindings/spi/spi-bus.txt. Use "num-cs"
> here.
> >
> >> +
> >> +Example:
> >> +
> >> + spi@e0007000 {
> >> + clock-names = "ref_clk", "pclk";
> >> + clocks = <&clkc 26>, <&clkc 35>;
> >> + compatible = "cdns,spi-r1p6";
> >
> > Nit. We usually put compatible first in the node.
>
> Our device-tree generator sorts them that's why it is just like this but not a
> problem to fix just in documentation.
>
Will fix.
>
> >> + interrupt-parent = <&intc>;
> >> + interrupts = <0 49 4>;
> >> + num-chip-select = /bits/ 16 <4>;
>
> I was expecting you will comment this a little bit. :-) Because all just
> reading
> this num-cs as 32bit and then assigning this value to master-
> >num_chipselect which is 16bit.
>
>
>
> >> +/* Macros for the SPI controller read/write */
> >> +#define cdns_spi_read(addr)readl_relaxed(addr)
> >> +#define cdns_spi_write(addr, val) writel_relaxed((val), (addr))
> >
> > Just use readl/writel directly.
>
> There shouldn't be any problem to use these helper macros and even I think
> this is better than using r
RE: [PATCH] SPI: Add driver for Cadence SPI controller
Hi Rob,
> -Original Message-
> From: Rob Herring [mailto:robherri...@gmail.com]
> Sent: Monday, March 17, 2014 6:17 PM
> To: Harini Katakam
> Cc: Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Rob
> Landley; Mark Brown; Grant Likely; devicet...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> s...@vger.kernel.org; Michal Simek
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> On Mon, Mar 17, 2014 at 7:05 AM, Harini Katakam
> wrote:
> > Add driver for Cadence SPI controller. This is used in Xilinx Zynq.
> >
> > Signed-off-by: Harini Katakam
> > ---
>
> > +- reg : Physical base address and size of SPI registers
> > map.
> > +- interrupts : Property with a value describing the interrupt
> > + number.
> > +- interrupt-parent : Must be core interrupt controller
> > +- clock-names : List of input clock names - "ref_clk", "pclk"
> > + (See clock bindings for details).
> > +- clocks : Clock phandles (see clock bindings for details).
> > +- num-chip-select : Number of chip selects used.
>
> See Documentation/devicetree/bindings/spi/spi-bus.txt. Use "num-cs"
> here.
I'll check that.
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
>
> I believe this returns NO_IRQ which could be 0.
>
> s/
> > + dev_dbg(&pdev->dev, "suspend succeeded\n");
>
> Not needed. The kernel already has suspend/resume tracing printks.
Will remove.
> > + dev_dbg(&pdev->dev, "resume succeeded\n");
>
> Not needed. The kernel already has suspend/resume tracing printks.
Will remove.
Regards,
Harini
This email and any attachments are intended for the sole use of the named
recipient(s) and contain(s) confidential information that may be proprietary,
privileged or copyrighted under applicable law. If you are not the intended
recipient, do not read, copy, or forward this email message or any attachments.
Delete this email message and any attachments immediately.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] SPI: Add driver for Cadence SPI controller
Hi Mark,
> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Monday, March 17, 2014 11:00 PM
> To: Harini Katakam
> Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; r...@landley.net;
> grant.lik...@linaro.org; devicet...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> s...@vger.kernel.org; Michal Simek
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> On Mon, Mar 17, 2014 at 05:35:36PM +0530, Harini Katakam wrote:
>
> > + bits_per_word = transfer ?
> > + transfer->bits_per_word : spi->bits_per_word;
>
> This would be a lot more legible without the ternery operator.
>
> > + if (bits_per_word != 8) {
> > + dev_err(&spi->dev, "%s, unsupported bits per word %x\n",
> > + __func__, spi->bits_per_word);
> > + return -EINVAL;
> > + }
>
> The core will check this for you.
I dint find that the core does this.
>
> > +static int cdns_spi_setup(struct spi_device *spi) {
> > + if (!spi->max_speed_hz)
> > + return -EINVAL;
> > +
> > + if (!spi->bits_per_word)
> > + spi->bits_per_word = 8;
>
> The core does this for you.
I understand that the core does this.
>
> > + return cdns_spi_setup_transfer(spi, NULL); }
>
> It's not clear to me why this has been split into a separate function and the
> function will write to the hardware which you're not allowed to do in
> setup() if it might affect an ongoing transfer.
Are you saying that there's a chance cdns_spi_setup() will be called when
there's an ongoing transfer?
In that case I have to remove the cdns_setup_transfer() call from here but then
there's nothing left to do in setup.
The function was split into two because cdns_spi_setup_transfer() is called
internally
by transfer_one_message() everytime.
Is it always possible that the spi_transfer paramters will remain the same;
In that case this call is probably not necessary in transfer_one_message.
>
> > + intr_status = cdns_spi_read(xspi->regs + CDNS_SPI_ISR_OFFSET);
> > + cdns_spi_write(xspi->regs + CDNS_SPI_ISR_OFFSET, intr_status);
> > +
> > + if (intr_status & CDNS_SPI_IXR_MODF_MASK) {
> > + /* Indicate that transfer is completed, the SPI subsystem will
> > +* identify the error as the remaining bytes to be
> > +* transferred is non-zero
> > +*/
> > + cdns_spi_write(xspi->regs + CDNS_SPI_IDR_OFFSET,
> > + CDNS_SPI_IXR_DEFAULT_MASK);
> > + complete(&xspi->done);
> > + } else if (intr_status & CDNS_SPI_IXR_TXOW_MASK) {
>
> What happens if both interrupts are flagged at the same time?
The MODF is an error interrupt and so if TXOW is raised along with it,
TXOW will be ignored (it will be cleared but no data is read).
Completion is indicated and since the remaining bytes is non-zero,
The transfer function will understand that it is an error.
>
> > + if (xspi->remaining_bytes) {
> > + /* There is more data to send */
> > + cdns_spi_fill_tx_fifo(xspi);
> > + } else {
> > + /* Transfer is completed */
> > + cdns_spi_write(xspi->regs + CDNS_SPI_IDR_OFFSET,
> > + CDNS_SPI_IXR_DEFAULT_MASK);
> > + complete(&xspi->done);
> > + }
> > + }
>
> I see from further up the file that there are error interrupt states which
> might be flagged but these are just being ignored.
I'm not sure I understand what you are referring to -
The only interrupts enabled (See CNDS_IXR_DEFAULT_MASK) are TXOW and MODF.
>
> > +
> > + return IRQ_HANDLED;
>
> This should only be returned if an interrupt was actually handled - if the
> line
> is shared in some system this will break.
In this case both possible interrupt conditions are handled.
>
> > + cdns_spi_write(xspi->regs + CDNS_SPI_IER_OFFSET,
> > + CDNS_SPI_IXR_DEFAULT_MASK);
> > +
> > + ret = wait_for_completion_interruptible_timeout(&xspi->done,
> > + CDNS_SPI_TIMEOUT);
> > + if (ret < 1) {
>
> If you return a positive value from transfer_one() the core will wait for you.
>
I don't understand. Here, if the ret from
wait_for_completion_interruptible_timeout is
positive, then this functi
RE: [PATCH] SPI: Add driver for Cadence SPI controller
Hi Mark,
> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Monday, March 17, 2014 11:45 PM
> To: Josh Cartwright
> Cc: Harini Katakam; robh...@kernel.org; pawel.m...@arm.com;
> mark.rutl...@arm.com; ijc+devicet...@hellion.org.uk;
> ga...@codeaurora.org; r...@landley.net; grant.lik...@linaro.org;
> devicet...@vger.kernel.org; linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; Michal Simek
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> On Mon, Mar 17, 2014 at 12:59:11PM -0500, Josh Cartwright wrote:
> > On Mon, Mar 17, 2014 at 05:30:17PM +, Mark Brown wrote:
> > > On Mon, Mar 17, 2014 at 05:35:36PM +0530, Harini Katakam wrote:
>
> > > > +static int __maybe_unused cdns_spi_suspend(struct device *dev) {
>
> > > This needs to call spi_master_suspend() as well (and similarly on
> > > resume).
>
> > I'm not that familiar with the SPI core, but this seems like an
> > inversion. Is there a reason why the SPI master class doesn't
> > implement
> > suspend/resume() callbacks which handle stopping/starting the queue
> > automatically for all masters?
>
> This is for users of an optional feature of the infrastructure. We probably
> should just call it anyway since it does have checks for the feature being
> used (but given all the open coding around this stuff I'd need to verify that
> the class callbacks would reliably get called).
>
> In any case that's not happening now and the driver as it stands is buggy
> since it's trampling all over the hardware without syncing with anything that
> isn't ongoing.
In case of a suspend, we are stopping an ongoing transfer and disabling the
interface.
In case I add clock disable and anything else to unprepared too, it will be a
cleaner
exit but it will still stop the transfer right? What do you suggest?
Should we wait for transfer to complete or a timeout to occur?
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] SPI: Add driver for Cadence SPI controller
Hi Mark,
> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Tuesday, March 18, 2014 4:34 PM
> To: Harini Katakam
> Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; r...@landley.net;
> grant.lik...@linaro.org; devicet...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> s...@vger.kernel.org; Michal Simek
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> On Tue, Mar 18, 2014 at 05:16:26AM +, Harini Katakam wrote:
>
> Please fix your mailer to word wrap within paragraphs, this will make
> your mail much more legible.
>
> > > > + if (bits_per_word != 8) {
> > > > + dev_err(&spi->dev, "%s, unsupported bits per word %x\n",
> > > > + __func__, spi->bits_per_word);
> > > > + return -EINVAL;
> > > > + }
>
> > > The core will check this for you.
>
> > I dint find that the core does this.
>
> bpw_mask.
>
OK. Will set master->bits_per_word_mask.
> > > It's not clear to me why this has been split into a separate function and
> the
> > > function will write to the hardware which you're not allowed to do in
> > > setup() if it might affect an ongoing transfer.
>
> > Are you saying that there's a chance cdns_spi_setup() will be called
> > when there's an ongoing transfer? In that case I have to remove the
> > cdns_setup_transfer() call from here but then there's nothing left to
> > do in setup.
>
> yes.
>
I'm going to remove the bits_per_word check anyway.
But the clock configuration still needs to be done.
Where should it be done spi_setup() or transfer?
Can you please comment on this?
"The function was split into two because cdns_spi_setup_transfer() is called
internally by transfer_one_message() everytime.
Is it always possible that the spi_transfer paramters will remain the same?
In that case this call is probably not necessary in transfer_one_message."
> > > I see from further up the file that there are error interrupt states which
> > > might be flagged but these are just being ignored.
>
> > I'm not sure I understand what you are referring to -
> > The only interrupts enabled (See CNDS_IXR_DEFAULT_MASK) are TXOW
> and MODF.
>
> The code had:
>
> > +#define CDNS_SPI_IXR_TXOW_MASK 0x0004 /* SPI TX FIFO
> Overwater */
> > +#define CDNS_SPI_IXR_MODF_MASK 0x0002 /* SPI Mode Fault */
> > +#define CDNS_SPI_IXR_RXNEMTY_MASK 0x0010 /* SPI RX FIFO Not
> Empty */
>
> > +#define CDNS_SPI_IXR_TXFULL_MASK 0x0008 /* SPI TX Full */
>
> and defined:
>
> > +#define CDNS_SPI_IXR_ALL_MASK 0x007F /* SPI all interrupts */
>
> > > > + return IRQ_HANDLED;
>
> > > This should only be returned if an interrupt was actually handled - if the
> line
> > > is shared in some system this will break.
>
> > In this case both possible interrupt conditions are handled.
>
> Are you sure that's the case, and even if you are that's still not
> handling the case where the device isn't flagging an interrupt at all.
>
The IXR_ALL mask is only used to disable all the interrupts in the beginning.
These two are the only interrupts enabled.
And RXNEMPTY status is just polled. That interrupt is not enabled either
Regards,
Harini
This email and any attachments are intended for the sole use of the named
recipient(s) and contain(s) confidential information that may be proprietary,
privileged or copyrighted under applicable law. If you are not the intended
recipient, do not read, copy, or forward this email message or any attachments.
Delete this email message and any attachments immediately.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] SPI: Add driver for Cadence SPI controller
HI Mark,
> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Tuesday, March 18, 2014 6:04 PM
> To: Harini Katakam
> Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; r...@landley.net;
> grant.lik...@linaro.org; devicet...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> s...@vger.kernel.org; Michal Simek
> Subject: Re: [PATCH] SPI: Add driver for Cadence SPI controller
>
> On Tue, Mar 18, 2014 at 12:13:45PM +, Harini Katakam wrote:
>
>
> > > > In this case both possible interrupt conditions are handled.
>
> > > Are you sure that's the case, and even if you are that's still not
> > > handling the case where the device isn't flagging an interrupt at all.
>
> > The IXR_ALL mask is only used to disable all the interrupts in the
> beginning.
> > These two are the only interrupts enabled.
> > And RXNEMPTY status is just polled. That interrupt is not enabled either
>
> This is all going to be fragile in the face of bugs or changes in the
> code though and like I keep saying it doesn't handle interrupt sharing.
OK. I dint consider interrupt sharing.
Do you think the following implementation would be better?
status = IRQ_NONE;
if (intr_status & CDNS_SPI_IXR_MODF_MASK) {
/* Handle this interrupt here */
status = IRQ_HANDLED;
} else if (intr_status & CDNS_SPI_IXR_TXOW_MASK) {
/* Handle this interrupt here */
status = IRQ_HANDLED;
}
return status;
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] SPI: Add driver for Cadence SPI controller
Add driver for Cadence SPI controller. This is used in Xilinx Zynq. Signed-off-by: Harini Katakam --- v2 changes: - Use xilinx compatible string too. - Changes read register and write register functions to static inline. - Removed unecessary dev_info and dev_dbg prints. - Return IRQ_HANDLED only when interrupt is handled. - Use a default num-cs value. - Do init_hardware before requesting irq. - Remove unecessary master_put() - Set master->max_speed_hz - Check for busy in cdns_setup(). Retained this function with this check as opposed to removing. The reason for this is clock configuration needs to be done for the first time before enable is done in prepare_hardware; prepare_hardware however, doesn't receive spi_device structure. - Implememnt transfer_one instead of transfer_one_message. Remove wait_for_completion as this is done by core. - Implement set_cs. - Clock enable/disable in prepare/unprepare respectively. - In suspend, remove reset of hardware; simply call unprepare_hardware. - In suspend/resume call master_suspend/resume respectively. - Check for irq<=0 in probe. - Remove MODULE_ALIAS. --- drivers/spi/Kconfig |7 + drivers/spi/Makefile |1 + drivers/spi/spi-cadence.c | 677 + 3 files changed, 685 insertions(+) create mode 100644 drivers/spi/spi-cadence.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 581ee2a..aeae44a 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -148,6 +148,13 @@ config SPI_BUTTERFLY inexpensive battery powered microcontroller evaluation board. This same cable can be used to flash new firmware. +config SPI_CADENCE + tristate "Cadence SPI controller" + depends on SPI_MASTER + help + This selects the Cadence SPI controller master driver + used by Xilinx Zynq. + config SPI_CLPS711X tristate "CLPS711X host SPI controller" depends on ARCH_CLPS711X diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 95af48d..1be2ed7 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_SPI_BFIN_V3) += spi-bfin-v3.o obj-$(CONFIG_SPI_BFIN_SPORT) += spi-bfin-sport.o obj-$(CONFIG_SPI_BITBANG) += spi-bitbang.o obj-$(CONFIG_SPI_BUTTERFLY)+= spi-butterfly.o +obj-$(CONFIG_SPI_CADENCE) += spi-cadence.o obj-$(CONFIG_SPI_CLPS711X) += spi-clps711x.o obj-$(CONFIG_SPI_COLDFIRE_QSPI)+= spi-coldfire-qspi.o obj-$(CONFIG_SPI_DAVINCI) += spi-davinci.o diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c new file mode 100644 index 000..071642d --- /dev/null +++ b/drivers/spi/spi-cadence.c @@ -0,0 +1,677 @@ +/* + * Cadence SPI controller driver (master mode only) + * + * Copyright (C) 2008 - 2014 Xilinx, Inc. + * + * based on Blackfin On-Chip SPI Driver (spi_bfin5xx.c) + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License version 2 as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Name of this driver */ +#define CDNS_SPI_NAME "cdns-spi" + +/* Register offset definitions */ +#define CDNS_SPI_CR_OFFSET 0x00 /* Configuration Register, RW */ +#define CDNS_SPI_ISR_OFFSET0x04 /* Interrupt Status Register, RO */ +#define CDNS_SPI_IER_OFFSET0x08 /* Interrupt Enable Register, WO */ +#define CDNS_SPI_IDR_OFFSET0x0c /* Interrupt Disable Register, WO */ +#define CDNS_SPI_IMR_OFFSET0x10 /* Interrupt Enabled Mask Register, RO */ +#define CDNS_SPI_ER_OFFSET 0x14 /* Enable/Disable Register, RW */ +#define CDNS_SPI_DR_OFFSET 0x18 /* Delay Register, RW */ +#define CDNS_SPI_TXD_OFFSET0x1C /* Data Transmit Register, WO */ +#define CDNS_SPI_RXD_OFFSET0x20 /* Data Receive Register, RO */ +#define CDNS_SPI_SICR_OFFSET 0x24 /* Slave Idle Count Register, RW */ +#define CDNS_SPI_THLD_OFFSET 0x28 /* Transmit FIFO Watermark Register,RW */ + +/* + * SPI Configuration Register bit Masks + * + * This register contains various control bits that affect the operation + * of the SPI controller + */ +#define CDNS_SPI_CR_MANSTRT_MASK 0x0001 /* Manual TX Start */ +#define CDNS_SPI_CR_CPHA_MASK 0x0004 /* Clock Phase Control */ +#define CDNS_SPI_CR_CPOL_MASK 0x0002 /* Clock Polarity Control */ +#define CDNS_SPI_CR_SSCTRL_MASK0x3C00 /* Slave Select Mask */ +#define CDNS_SPI_CR_BAUD_DIV_MASK 0x0038 /* Baud Rate Divisor Mask */ +#define CDNS_SPI_CR_MSTREN_MASK0x0001 /* Master Enable Mask */ +#define CDNS_SPI_CR_MANSTRTEN_MASK 0x8000 /* Manual TX Enable
[PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Add spi-cadence bindings documentation.
Signed-off-by: Harini Katakam
---
v2 changes:
- Separate patch for bindings.
- Add xilinx compatible string; Make compatible string first in the node.
- Use property name num-cs. Make this property optional.
---
.../devicetree/bindings/spi/spi-cadence.txt| 27
1 file changed, 27 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-cadence.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-cadence.txt
b/Documentation/devicetree/bindings/spi/spi-cadence.txt
new file mode 100644
index 000..ccb7599
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-cadence.txt
@@ -0,0 +1,27 @@
+Cadence SPI controller Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "cdns,spi-r1p6" or "xlnx,zynq-spi-r1p6".
+- reg : Physical base address and size of SPI registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- clock-names : List of input clock names - "ref_clk", "pclk"
+ (See clock bindings for details).
+- clocks : Clock phandles (see clock bindings for details).
+
+Optional properties:
+- num-cs : Number of chip selects used.
+
+Example:
+
+ spi@e0007000 {
+ compatible = "xlnx,zynq-spi-r1p6";
+ clock-names = "ref_clk", "pclk";
+ clocks = <&clkc 26>, <&clkc 35>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 49 4>;
+ num-cs = /bits/ 16 <4>;
+ reg = <0xe0007000 0x1000>;
+ } ;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] devicetree: Add devicetree bindings documentation for Zynq Quad SPI
Hi Soren
On Thu, Apr 3, 2014 at 11:20 PM, Sören Brinkmann
wrote:
> Hi Punnaiah,
>
> On Thu, 2014-04-03 at 10:33PM +0530, Punnaiah Choudary Kalluri wrote:
>> Add bindings documentation for Zynq Quad SPI driver.
>>
>> Signed-off-by: Punnaiah Choudary Kalluri
>> ---
>> .../devicetree/bindings/spi/spi-zynq-qspi.txt | 26
>>
>> 1 file changed, 26 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
>> b/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
>> new file mode 100644
>> index 000..88e00f8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
>> @@ -0,0 +1,26 @@
>> +Xilinx Zynq QSPI controller Device Tree Bindings
>> +-
>> +
>> +Required properties:
>> +- compatible : Should be "xlnx,zynq-qspi-1.0".
>> +- reg: Physical base address and size of QSPI
>> registers map.
>> +- interrupts : Property with a value describing the interrupt
>> + number.
>> +- interrupt-parent : Must be core interrupt controller
>> +- clock-names: List of input clock names - "ref_clk",
>> "aper_clk"
>> + (See clock bindings for details).
>> +- clocks : Clock phandles (see clock bindings for details).
>> +
>> +Optional properties:
>> +- num-cs : Number of chip selects used.
>> +
>> +Example:
>> + qspi@e000d000 {
>> + compatible = "xlnx,zynq-qspi-1.0";
>> + clock-names = "ref_clk", "aper_clk";
>
> These seem to be the SOC names of the clocks. Doesn't have the IP its
> own naming for these clock inputs?
>
The IP design spec uses the name ref_clk.
There is no particular clock name used for for APB clock.
So I think aper_clk is a valid name to use.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi Mark, On Fri, Apr 4, 2014 at 3:04 AM, Mark Brown wrote: > On Thu, Apr 03, 2014 at 04:40:31PM +0530, Harini Katakam wrote: > >> +Optional properties: >> +- num-cs : Number of chip selects used. > > How does this translate to the hardware? This IP can drive 4 slaves. The CS line to be driven is selected in spi device structure and that is driven by the software. > >> + num-cs = /bits/ 16 <4>; > > What's going on with the /bits/ - is this something that's required for > the property? The master->num-chipselect property is 16 bit but writing <4> here directly leads to 0 being read in of_property_read (because it's big endian). Instead using of property read u32 and then copying, we decided to do this. This was discussed on v2 between Michal and Rob: >>>> + num-chip-select = /bits/ 16 <4>; >> >> I was expecting you will comment this a little bit. :-) >> Because all just reading this num-cs as 32bit and then >> assigning this value to master->num_chipselect which is 16bit. > > Well, everyone else has that problem then. Obviously it takes a bit > more care than just reading into a u32, but that is a kernel problem > and not a problem of the binding. They are not reading it directly with read_u32 but they are using intermediate u32 value which is assigned to u16 which is fine. This pattern is in most drivers(maybe all). The point is if binding should or can't simplify driver code. And from your reaction above I expect that it is up to driver owner and binding doc how you want to do it. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] devicetree: Add devicetree bindings documentation for Zynq Quad SPI
Hi Mark, On Fri, Apr 4, 2014 at 2:31 AM, Mark Brown wrote: > On Thu, Apr 03, 2014 at 10:33:06PM +0530, Punnaiah Choudary Kalluri wrote: > >> +Optional properties: >> +- num-cs : Number of chip selects used. > > What does this translate into? > >> + num-cs = /bits/ 16 <1>; > > Why the odd specification in the example - why not just specify it as a > number? Same as discussed on SPI cadence thread. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] SPI: Add support for Zynq Quad SPI controller
Hi Mark,
On Fri, Apr 4, 2014 at 2:59 AM, Mark Brown wrote:
> On Thu, Apr 03, 2014 at 10:33:07PM +0530, Punnaiah Choudary Kalluri wrote:
>
> Overall this looks fairly good, there are a few issues that need to be
> looked at but they're not too invasive. Please also check for coding
> style issues, quite a few spaces before commas for example.
>
Thanks. I'll check that.
>> +/**
>> + * zynq_qspi_copy_read_data - Copy data to RX buffer
>> + * @xqspi: Pointer to the zynq_qspi structure
>> + * @data:The 32 bit variable where data is stored
>> + * @size:Number of bytes to be copied from data to RX buffer
>> + */
>> +static void zynq_qspi_copy_read_data(struct zynq_qspi *xqspi, u32 data, u8
>> size)
>> +{
>> + if (xqspi->rxbuf) {
>> + memcpy(xqspi->rxbuf, ((u8 *) &data) + 4 - size, size);
>> + xqspi->rxbuf += size;
>> + }
>> + xqspi->bytes_to_receive -= size;
>> +}
>
> Does this and the write function really need to be a separate function -
> it's trivial and used once? It's probably more beneficial to split out
> some of the more complex logic later on that's causing the indentation
> to get too deep.
>
I'm aware it's used in only one place but it does make receive data handling
easier for future. As you may have noticed there are 4 different ways to
write into transmit FIFO and the data read also differs accordingly.
I'll try to reduce the indentation in other places.
>> +static int zynq_qspi_setup_transfer(struct spi_device *qspi,
>> + struct spi_transfer *transfer)
>> +{
>> + struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
>> + u32 config_reg, req_hz, baud_rate_val = 0;
>> +
>> + if (transfer)
>> + req_hz = transfer->speed_hz;
>> + else
>> + req_hz = qspi->max_speed_hz;
>
> Why would a transfer be being set up without a transfer being provided?
>
The setup function calls this function before a transfer is initiated.
In this case NULL is passed to setup_transfer (see below) and
SPI is initialized with default clock configuration.
This initialization is necessary because otherwise this clock config
would be done
only after SPI is enabled in prepare_hardware, which is wrong.
(I'm checking for master->busy in setup to address your previous
comment on SPI).
I explained the same in SPI v2 changes and this valid there too.
>> +/**
>> + * zynq_qspi_setup - Configure the QSPI controller
>> + * @qspi:Pointer to the spi_device structure
>> + *
>> + * Sets the operational mode of QSPI controller for the next QSPI transfer,
>> baud
>> + * rate and divisor value to setup the requested qspi clock.
>> + *
>> + * Return: 0 on success and error value on failure
>> + */
>> +static int zynq_qspi_setup(struct spi_device *qspi)
>> +{
>> + if (qspi->master->busy)
>> + return -EBUSY;
>> +
>> + return zynq_qspi_setup_transfer(qspi, NULL);
>> +}
>
> No, this is broken - you have to support setup() while the hardware is
> active. Just remove this if there's nothing to do and set up on the
> transfer.
But where do you suggest this clock configuration be done?
I've looked at the option of doing it in prepare_hardware but
spi_device structure is not passed to it.
>
>> + if (xqspi->rxbuf) {
>> + (*(u32 *)xqspi->rxbuf) =
>> + zynq_qspi_read(xqspi,
>> +ZYNQ_QSPI_RXD_OFFSET);
>> + xqspi->rxbuf += 4;
>
> This only works in 4 byte words? That seems a bit limited.
> Alternatively, if it works with smaller sizes (as it appears to) then
> isn't this at risk of overflowing buffers?
>
There is a
if (xqspi->bytes_to_receive < 4) {
above and this statement is in the else loop.
When less than 4 bytes are being read/received, the handling is different.
>> +static int __maybe_unused zynq_qspi_suspend(struct device *_dev)
>> +{
>> + struct platform_device *pdev = container_of(_dev,
>> + struct platform_device, dev);
>> + struct spi_master *master = platform_get_drvdata(pdev);
>> +
>> + spi_master_suspend(master);
>> +
>> + zynq_unprepare_transfer_hardware(master);
>
> Why are you unpreparing the hardware - the framework should be doing
> that for you if the device is active, if it's not you've got an extra
> clock disable here?
>
I called unprepare_hardware becuase it does the things necessary
after master suspend - disable clock and controller.
(I thought this was your suggestion for SPI?)
>> +static int __maybe_unused zynq_qspi_resume(struct device *dev)
>
> This doesn't appear to be calling init_hw() - is it guaranteed that all
> the register settings written there are OK after power on?
>
>> + ret = clk_prepare_enable(xqspi->aperclk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Unable to enable APER clock.\n");
>> + go
Re: [PATCH v2 1/2] SPI: Add driver for Cadence SPI controller
Hi Mark On Fri, Apr 4, 2014 at 3:13 AM, Mark Brown wrote: > On Thu, Apr 03, 2014 at 04:40:30PM +0530, Harini Katakam wrote: >> Add driver for Cadence SPI controller. This is used in Xilinx Zynq. > > I just reviewed a driver for "Zynq Quad SPI controller" from Punnaiah > Choudary Kalluri (CCed) which seems *very* similar to this one. Are > there opportunities for code sharing here (I'm not entirely sure the > hardware blocks are different, though I didn't check in detail). > Thanks for the review. QSPI is a Xilinx IP built on top of cadence SPI with considerable functional changes. As explained in the QSPI patch, there are three configurations QSPI supports : - A single flash device connected with 1 CS and 4 IO lines - Two flash devices connected over two separate sets of 4 IO lines and two CS lines which are driven together. - Two flash devices connected with two separate CS line and one common set of 4 IO lines. This first set of QSPI patches is only for the single flash configuration. As the next two configurations follow, QSPI driver will differ from SPI even more. That's why it might be better to have two separate drivers. It will avoid a lot of "if spi/ if qspi" checks. I will send an RFC with proposed changes for all QSPI configurations. Also, I've replied to your comments on the QSPI driver. (The QSPI driver already addresses the comments for SPI v1) Except in two places where the comment was only applicable to QSPI driver, the replies hold good for both SPI and QSPI drivers. If you would like to continue the discussion on that thread, I'm ok with it. FYI, I'll be sending the next versions for both drivers after further discussion concludes. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi Mark, On Fri, Apr 4, 2014 at 3:39 PM, Mark Brown wrote: > On Fri, Apr 04, 2014 at 08:30:38AM +0530, Harini Katakam wrote: >> On Fri, Apr 4, 2014 at 3:04 AM, Mark Brown wrote: >> > On Thu, Apr 03, 2014 at 04:40:31PM +0530, Harini Katakam wrote: > >> >> +Optional properties: >> >> +- num-cs : Number of chip selects used. > >> > How does this translate to the hardware? > >> This IP can drive 4 slaves. >> The CS line to be driven is selected in spi device structure and >> that is driven by the software. > > So why specify this in the binding at all? If the device always has the > same number of chip selects it's redundant. I'm sorry, I should have explained that better. The IP can support upto 4 chip selects. The num-cs value here is the number of chip selects actually used on the board. > >> >> + num-cs = /bits/ 16 <4>; > >> > What's going on with the /bits/ - is this something that's required for >> > the property? > >> The master->num-chipselect property is 16 bit but writing <4> here directly >> leads to 0 being read in of_property_read (because it's big endian). >> Instead using of property read u32 and then copying, we decided to do this. >> This was discussed on v2 between Michal and Rob: > > No, don't do that. You're contorting the binding to serve Linux's > implementation needs and you've not documented this requirement either. > If there *was* a need to have the number be 16 bits you'd need to > document that reqirement in the binding, though like I say I don't think > the number needs to be there at all. OK. I'll remove the /bits/ 16 and handle it in the driver. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] SPI: Add support for Zynq Quad SPI controller
Hi Mark,
On Fri, Apr 4, 2014 at 4:38 PM, Mark Brown wrote:
> On Fri, Apr 04, 2014 at 08:59:47AM +0530, Harini Katakam wrote:
>> On Fri, Apr 4, 2014 at 2:59 AM, Mark Brown wrote:
>
>> > Why would a transfer be being set up without a transfer being provided?
>
>> The setup function calls this function before a transfer is initiated.
>> In this case NULL is passed to setup_transfer (see below) and
>> SPI is initialized with default clock configuration.
>> This initialization is necessary because otherwise this clock config
>> would be done
>> only after SPI is enabled in prepare_hardware, which is wrong.
>> (I'm checking for master->busy in setup to address your previous
>> comment on SPI).
>
> The requirement for setup() to work when other transfers are in progress
> is clear and unambiguous, it really isn't acceptable to reconfigure
> hardware in use by a runing transfer in setup().
>
OK. I'll remove setup_transfer here and handle clock configuration elsewhere.
>> I explained the same in SPI v2 changes and this valid there too.
>
> This is v2?
>
No. This "Zynq QSPI" patch is v1.
I was referring to "Cadence SPI" v2 patch in which you pointed to these
comments.
Sorry for the confusion.
>> >> +static int __maybe_unused zynq_qspi_suspend(struct device *_dev)
>> >> +{
>> >> + struct platform_device *pdev = container_of(_dev,
>> >> + struct platform_device, dev);
>> >> + struct spi_master *master = platform_get_drvdata(pdev);
>> >> +
>> >> + spi_master_suspend(master);
>> >> +
>> >> + zynq_unprepare_transfer_hardware(master);
>
>> > Why are you unpreparing the hardware - the framework should be doing
>> > that for you if the device is active, if it's not you've got an extra
>> > clock disable here?
>
>> I called unprepare_hardware becuase it does the things necessary
>> after master suspend - disable clock and controller.
>> (I thought this was your suggestion for SPI?)
>
> Why are these things required after the core has already idled the
> device (using exactly the same function)?
Ok. It's unecessary I'll remove it.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi, On Fri, Apr 4, 2014 at 5:54 PM, Peter Crosthwaite wrote: > On Fri, Apr 4, 2014 at 10:14 PM, Harini Katakam > wrote: >> Hi Mark, >> >> On Fri, Apr 4, 2014 at 3:39 PM, Mark Brown wrote: >>> On Fri, Apr 04, 2014 at 08:30:38AM +0530, Harini Katakam wrote: >>>> On Fri, Apr 4, 2014 at 3:04 AM, Mark Brown wrote: >>>> > On Thu, Apr 03, 2014 at 04:40:31PM +0530, Harini Katakam wrote: >>> >>>> >> +Optional properties: >>>> >> +- num-cs : Number of chip selects used. >>> >>>> > How does this translate to the hardware? >>> >>>> This IP can drive 4 slaves. >>>> The CS line to be driven is selected in spi device structure and >>>> that is driven by the software. >>> >>> So why specify this in the binding at all? If the device always has the >>> same number of chip selects it's redundant. >> >> I'm sorry, I should have explained that better. >> The IP can support upto 4 chip selects. >> The num-cs value here is the number of chip selects actually used on the >> board. >> > > Just to clarify - do you mean SoC or board level? > I mean board level. One more reason it will be useful to keep this property is because there is support for adding a decoder where extended slaves can be selected through the IP's control register. (This is not currently implemented in the driver but will be in the future.) Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi Mark, On Fri, Apr 4, 2014 at 6:16 PM, Mark Brown wrote: > On Fri, Apr 04, 2014 at 05:44:23PM +0530, Harini Katakam wrote: >> On Fri, Apr 4, 2014 at 3:39 PM, Mark Brown wrote: >> > On Fri, Apr 04, 2014 at 08:30:38AM +0530, Harini Katakam wrote: > >> >> This IP can drive 4 slaves. >> >> The CS line to be driven is selected in spi device structure and >> >> that is driven by the software. > >> > So why specify this in the binding at all? If the device always has the >> > same number of chip selects it's redundant. > >> I'm sorry, I should have explained that better. >> The IP can support upto 4 chip selects. >> The num-cs value here is the number of chip selects actually used on the >> board. > > Why does that need to be configured? Surely the presence of slaves is > enough information. OK. I understand. Can you comment on the case where a decoder is used? There is support for adding a decoder where extended slaves can be selected through the IP's control register. (This is not currently implemented in the driver but will be in the future.) How should the driver know whether it is 4 or 16 select lines for example? This has to be written to master->num_chipselect. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi, On Fri, Apr 4, 2014 at 7:38 PM, Harini Katakam wrote: > Hi Mark, > > On Fri, Apr 4, 2014 at 6:16 PM, Mark Brown wrote: >> On Fri, Apr 04, 2014 at 05:44:23PM +0530, Harini Katakam wrote: >>> On Fri, Apr 4, 2014 at 3:39 PM, Mark Brown wrote: >>> > On Fri, Apr 04, 2014 at 08:30:38AM +0530, Harini Katakam wrote: >> >>> >> This IP can drive 4 slaves. >>> >> The CS line to be driven is selected in spi device structure and >>> >> that is driven by the software. >> >>> > So why specify this in the binding at all? If the device always has the >>> > same number of chip selects it's redundant. >> >>> I'm sorry, I should have explained that better. >>> The IP can support upto 4 chip selects. >>> The num-cs value here is the number of chip selects actually used on the >>> board. >> >> Why does that need to be configured? Surely the presence of slaves is >> enough information. > > OK. I understand. > Can you comment on the case where a decoder is used? > > There is support for adding a decoder where extended slaves can be selected > through the IP's control register. > (This is not currently implemented in the driver but will be in the future.) > > How should the driver know whether it is 4 or 16 select lines for example? > This has to be written to master->num_chipselect. > > Regards, > Harini Just adding to my above comment. Alternately, I could not use the "num-cs" property and add another optional property to be used only when required. The driver sets a default value already. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi Mark, On Fri, Apr 4, 2014 at 8:12 PM, Mark Brown wrote: > On Fri, Apr 04, 2014 at 07:38:14PM +0530, Harini Katakam wrote: > >> OK. I understand. >> Can you comment on the case where a decoder is used? > >> There is support for adding a decoder where extended slaves can be selected >> through the IP's control register. >> (This is not currently implemented in the driver but will be in the future.) > >> How should the driver know whether it is 4 or 16 select lines for example? >> This has to be written to master->num_chipselect. > > That's the sort of case where it's useful yes - depending on how the > implementation is done it may be sensible to just have a property > specifying if a decoder is there (if it just changes the encoding in the > register for example). OK, great. That approach will work. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi Peter, On Sat, Apr 5, 2014 at 4:44 AM, Peter Crosthwaite wrote: > On Sat, Apr 5, 2014 at 12:30 AM, Harini Katakam > wrote: >> Hi, >> >> On Fri, Apr 4, 2014 at 7:38 PM, Harini Katakam >> wrote: >>> Hi Mark, >>> >>> On Fri, Apr 4, 2014 at 6:16 PM, Mark Brown wrote: >>>> On Fri, Apr 04, 2014 at 05:44:23PM +0530, Harini Katakam wrote: >>>>> On Fri, Apr 4, 2014 at 3:39 PM, Mark Brown wrote: >>>>> > On Fri, Apr 04, 2014 at 08:30:38AM +0530, Harini Katakam wrote: >>>> >>>>> >> This IP can drive 4 slaves. >>>>> >> The sCS line to be driven is selected in spi device structure and >>>>> >> that is driven by the software. >>>> >>>>> > So why specify this in the binding at all? If the device always has the >>>>> > same number of chip selects it's redundant. >>>> >>>>> I'm sorry, I should have explained that better. >>>>> The IP can support upto 4 chip selects. >>>>> The num-cs value here is the number of chip selects actually used on the >>>>> board. > > Shouldnt it be a property of the controller not the board? How for > example do we differentiate between different implementations of > cadence SPI controller that only supports up to two CS lines? My > thinking is that this property should always be present and = 4 in the > Zynq case as the controller always has 4 physical CS lines coming out > (before any decoders, pin muxes or slaves etc.). > >>>> >>>> Why does that need to be configured? Surely the presence of slaves is >>>> enough information. > > And presence of slaves / board info is inferable from subnodes. > >>> >>> OK. I understand. >>> Can you comment on the case where a decoder is used? >>> >>> There is support for adding a decoder where extended slaves can be selected >>> through the IP's control register. >>> (This is not currently implemented in the driver but will be in the future.) >>> > > I think thats seperate information. is-decoded-cs property of something. > >>> How should the driver know whether it is 4 or 16 select lines for example? >>> This has to be written to master->num_chipselect. >>> > > If you only have one property (== 4) how do you tell the difference > between 4 un-decoded CS lines vs 2 decoded CS lines? > If an SoC implements 2 CS and sets "decoder" property, then we'd configure the register with "select decode" bit and write 0b0011 to the slave select field to select 4th slave. If decoder property is not set, then we'd write 0b1000 to select 4th slave. But yes, if the SoC only implements 2 CS, doesn't use decoder but if there is an erroneous input to set_cs for 4th slave, it would be a problem. > > I think num-cs has validity as the number of CS lines implemented in > the controller. For any given SoC, it's constant but could differ > between SoCs. > I dint consider that this property will be useful to SoC's implementing <4 CS; master->num_chipselect (when set to this property's value) will be used to check error conditions. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Hi Peter, On Sun, Apr 6, 2014 at 5:13 AM, Peter Crosthwaite wrote: > On Sat, Apr 5, 2014 at 4:05 PM, Harini Katakam > wrote: >> Hi Peter, >> >> On Sat, Apr 5, 2014 at 4:44 AM, Peter Crosthwaite >> wrote: >>> On Sat, Apr 5, 2014 at 12:30 AM, Harini Katakam >>> wrote: >>>> Hi, >>>> >>>> On Fri, Apr 4, 2014 at 7:38 PM, Harini Katakam >>>> wrote: >>>>> Hi Mark, >>>>> >>>>> On Fri, Apr 4, 2014 at 6:16 PM, Mark Brown wrote: >>>>>> On Fri, Apr 04, 2014 at 05:44:23PM +0530, Harini Katakam wrote: >>>>>>> On Fri, Apr 4, 2014 at 3:39 PM, Mark Brown wrote: >>>>>>> > On Fri, Apr 04, 2014 at 08:30:38AM +0530, Harini Katakam wrote: >>>>>> >>>>>>> >> This IP can drive 4 slaves. >>>>>>> >> The sCS line to be driven is selected in spi device structure and >>>>>>> >> that is driven by the software. >>>>>> >>>>>>> > So why specify this in the binding at all? If the device always has >>>>>>> > the >>>>>>> > same number of chip selects it's redundant. >>>>>> >>>>>>> I'm sorry, I should have explained that better. >>>>>>> The IP can support upto 4 chip selects. >>>>>>> The num-cs value here is the number of chip selects actually used on >>>>>>> the board. >>> >>> Shouldnt it be a property of the controller not the board? How for >>> example do we differentiate between different implementations of >>> cadence SPI controller that only supports up to two CS lines? My >>> thinking is that this property should always be present and = 4 in the >>> Zynq case as the controller always has 4 physical CS lines coming out >>> (before any decoders, pin muxes or slaves etc.). >>> >>>>>> >>>>>> Why does that need to be configured? Surely the presence of slaves is >>>>>> enough information. >>> >>> And presence of slaves / board info is inferable from subnodes. >>> >>>>> >>>>> OK. I understand. >>>>> Can you comment on the case where a decoder is used? >>>>> >>>>> There is support for adding a decoder where extended slaves can be >>>>> selected >>>>> through the IP's control register. >>>>> (This is not currently implemented in the driver but will be in the >>>>> future.) >>>>> >>> >>> I think thats seperate information. is-decoded-cs property of something. >>> >>>>> How should the driver know whether it is 4 or 16 select lines for example? >>>>> This has to be written to master->num_chipselect. >>>>> >>> >>> If you only have one property (== 4) how do you tell the difference >>> between 4 un-decoded CS lines vs 2 decoded CS lines? >>> >> >> If an SoC implements 2 CS and sets "decoder" property, >> then we'd configure the register with "select decode" bit and write >> 0b0011 to the slave select field to select 4th slave. >> >> If decoder property is not set, then we'd write 0b1000 to select 4th slave. >> > > What are your proposed dt bindings for these differing options - what > is num-cs in each case? I think it will be better to have num-cs equal to the max slaves allowed and the property to indicate if decoder is present helps driver configure the register accordingly. (master->num_chipselect is written with "num-cs" value) For ex., 1. 4 slaves without decoder num-cs = <4> is-decoded-cs = 2. 4 slaves driven through 2 to 4 decoder: num-cs = <4> is-decoded-cs = The other option is have num-cs reflect number of CS before decoder but the above option makes more sense as num-cs directly reflects the max valid slaves. > >> But yes, if the SoC only implements 2 CS, doesn't use decoder but >> if there is an erroneous input to set_cs for 4th slave, it would be a >> problem. >> > > Sure, that's an error condition though that should be caught by SPI > because master->num_chipselect would only be 2 right? > Yes. Regards, Harini > Regards, > Peter > >>> >>> I think num-cs has validity as the number of CS lines implemented in >>> the controller. For any given SoC, it's constant but could differ >>> between SoCs. >>> >> >> I dint consider that this property will be useful to SoC's implementing <4 >> CS; >> master->num_chipselect (when set to this property's value) >> will be used to check error conditions. >> >> Regards, >> Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 2/2] devicetree: Add Cadence WDT devicetree bindings documentation
Add cadence-wdt bindings documentation.
Signed-off-by: Harini Katakam
---
v4 changes:
- Change name of property "reset" to "reset-on-timeout".
- Use cdns compatible string in example.
- Improve description of clocks and interrupts.
v3 changes:
- Change reset property type and improve description.
- Improve description of clocks and interrupts.
- Use watchdog@ in the example.
- Use only cdns compatible string for now.
v2:
No changes
---
.../devicetree/bindings/watchdog/cadence-wdt.txt | 24
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
new file mode 100644
index 000..c3a36ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
@@ -0,0 +1,24 @@
+Zynq Watchdog Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "cdns,wdt-r1p2".
+- clocks : This is pclk (APB clock).
+- interrupts : This is wd_irq - watchdog timeout interrupt.
+- interrupt-parent : Must be core interrupt controller.
+
+Optional properties
+- reset-on-timeout : If this property exists, then a reset is done
+ when watchdog times out.
+- timeout-sec : Watchdog timeout value (in seconds).
+
+Example:
+ watchdog@f8005000 {
+ compatible = "cdns,wdt-r1p2";
+ clocks = <&clkc 45>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 9 1>;
+ reg = <0xf8005000 0x1000>;
+ reset-on-timeout;
+ timeout-sec = <10>;
+ };
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/2] watchdog: Add Cadence WDT driver
Add Cadence WDT driver. This is used by Xilinx Zynq. Signed-off-by: Harini Katakam --- v4 changes: - Change name of property "reset" to "reset-on-timeout". v3 changes: - Make rst a boolean and use of_property_read_boolean. - Use only cdns compatible string. If customization is required in the future, xlnx compatible string can be added. v2 changes: - Update IO helpers. - Use dev_dbg instead of dev_info where possible. - Use watchdog_init_timeout - Spinlock for restart register where missing. - Change order of probe to move reboot notifier register to the end - Remove unecessary comments - Do clk_prepare_enabel and clk_disable_unprepare in resume/suspend respectively. - There is an input from dts to decide whether internal reset should be enabled or not. When this is enabled, reset happens wutomatically when counter reaches zero. In case this is not enabled, a message is disaplayed to indicate that the watchdog has timed out. Elaborated this message to describe the above. --- drivers/watchdog/Kconfig |7 + drivers/watchdog/Makefile |1 + drivers/watchdog/cadence_wdt.c | 516 3 files changed, 524 insertions(+) create mode 100644 drivers/watchdog/cadence_wdt.c diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 76dd541..adf5110 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -147,6 +147,13 @@ config AT91SAM9X_WATCHDOG Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will reboot your system when the timeout is reached. +config CADENCE_WATCHDOG + tristate "Cadence Watchdog Timer" + select WATCHDOG_CORE + help + Say Y here if you want to include support for the watchdog + timer in the Xilinx Zynq. + config 21285_WATCHDOG tristate "DC21285 watchdog" depends on FOOTBRIDGE diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 468c320..bae2fb0 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -32,6 +32,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o +obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o obj-$(CONFIG_TWL4030_WATCHDOG) += twl4030_wdt.o obj-$(CONFIG_21285_WATCHDOG) += wdt285.o diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c new file mode 100644 index 000..5927c0a --- /dev/null +++ b/drivers/watchdog/cadence_wdt.c @@ -0,0 +1,516 @@ +/* + * Cadence WDT driver - Used by Xilinx Zynq + * + * Copyright (C) 2010 - 2014 Xilinx, Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define CDNS_WDT_DEFAULT_TIMEOUT 10 +/* Supports 1 - 516 sec */ +#define CDNS_WDT_MIN_TIMEOUT 1 +#define CDNS_WDT_MAX_TIMEOUT 516 + +/* Restart key */ +#define CDNS_WDT_RESTART_KEY 0x1999 + +/* Counter register access key */ +#define CDNS_WDT_REGISTER_ACCESS_KEY 0x0092 + +/* Counter value divisor */ +#define CDNS_WDT_COUNTER_VALUE_DIVISOR 0x1000 + +/* Clock prescaler value and selection */ +#define CDNS_WDT_PRESCALE_64 64 +#define CDNS_WDT_PRESCALE_512 512 +#define CDNS_WDT_PRESCALE_4096 4096 +#define CDNS_WDT_PRESCALE_SELECT_641 +#define CDNS_WDT_PRESCALE_SELECT_512 2 +#define CDNS_WDT_PRESCALE_SELECT_4096 3 + +/* Input clock frequency */ +#define CDNS_WDT_CLK_10MHZ 1000 +#define CDNS_WDT_CLK_75MHZ 7500 + +/* Counter maximum value */ +#define CDNS_WDT_COUNTER_MAX 0xFFF + +static int wdt_timeout = CDNS_WDT_DEFAULT_TIMEOUT; +static int nowayout = WATCHDOG_NOWAYOUT; + +module_param(wdt_timeout, int, 0); +MODULE_PARM_DESC(wdt_timeout, +"Watchdog time in seconds. (default=" +__MODULE_STRING(CDNS_WDT_DEFAULT_TIMEOUT) ")"); + +module_param(nowayout, int, 0); +MODULE_PARM_DESC(nowayout, +"Watchdog cannot be stopped once started (default=" +__MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); + +/** + * struct cdns_wdt - Watchdog device structure + * @regs: baseaddress of device + * @rst: reset flag + * @clk: struct clk * of a clock source + * @prescaler: for saving prescaler value + * @ctrl_clksel: counter clock prescaler selection + * @io_lock: spinlock for IO register access + * @cdns_wdt_device: watchdog device structure + * @cdns_wdt_notifier: notifier structure + * + * Structure containing parameters specific to cadence watchdog. + */ +struct cdns_
[PATCH v3 1/2] SPI: Add driver for Cadence SPI controller
Add driver for Cadence SPI controller. This is used in Xilinx Zynq. Signed-off-by: Harini Katakam --- Here is the v3 series but I have one concern. The recent change in spi-core to use wait_for_completion_timeout uses a timeout value calculated as follows: ms = xfer->len * 8 * 1000/xfer->speed_hz; wait_for_completion_timeout(msecs_to_jiffies(ms)); Firstly, the timeout value obtained from this is a too low. This timeout is typically used for an error conditions such as hardware hang etc. and using a value >1*HZ would be better. This driver used to use similar timeout and as I understand, other drivers also use similar values. Second, the xfer->speed_hz used here is not necessarily the actual speed set. The IP allows a limited set of divisors and the divisor is chosen in driver such that the actual speed is <= requested speed. Due to this the timeout value set here, sometimes falls short of the actual time required to complete transfer; this happens especially with very low frequencies. The difference in time higher than 20msecs. And for very small transfers, the timeout is set to 1 jiffy. Also, this is not considering any interrupt latencies or if the system is fully loaded. This might be cutting it too close, can the timeout value in core be increased? v3 changes: - Remove setup function. Make clock CPOL/CPHA setup a separate funtion and call this from prepare_transfer_hardware before enabling. - Remove clk_enable/disable in prepare/unprepare_tranfer_hardware. Clock enable/diable pairs will be retained as before only in probe/remove and suspend/resume. Use of runtime_pms for efficiency will be planned later. - Remove /bits/ in bindings for num-cs, use temporary u32 variable in driver. - Rename requested_bytes and remaining_bytes to tx_bytes/rx_bytes. - Remove unused macros. - Add is-decoded-cs property and add support in driver set_cs funtion. v2 changes: - Use xilinx compatible string too. - Changes read register and write register functions to static inline. - Removed unecessary dev_info and dev_dbg prints. - Return IRQ_HANDLED only when interrupt is handled. - Use a default num-cs value. - Do init_hardware before requesting irq. - Remove unecessary master_put() - Set master->max_speed_hz - Check for busy in cdns_setup(). Retained this function with this check as opposed to removing. The reason for this is clock configuration needs to be done for the first time before enable is done in prepare_hardware; prepare_hardware however, doesn't receive spi_device structure. - Implememnt transfer_one instead of transfer_one_message. Remove wait_for_completion as this is done by core. - Implement set_cs. - Clock enable/disable in prepare/unprepare respectively. - In suspend, remove reset of hardware; simply call unprepare_hardware. - In suspend/resume call master_suspend/resume respectively. - Check for irq<=0 in probe. - Remove MODULE_ALIAS. --- drivers/spi/Kconfig |7 + drivers/spi/Makefile |1 + drivers/spi/spi-cadence.c | 673 + 3 files changed, 681 insertions(+) create mode 100644 drivers/spi/spi-cadence.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index efe1960..b0f091b 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -148,6 +148,13 @@ config SPI_BUTTERFLY inexpensive battery powered microcontroller evaluation board. This same cable can be used to flash new firmware. +config SPI_CADENCE + tristate "Cadence SPI controller" + depends on SPI_MASTER + help + This selects the Cadence SPI controller master driver + used by Xilinx Zynq. + config SPI_CLPS711X tristate "CLPS711X host SPI controller" depends on ARCH_CLPS711X || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index bd79266..3e503d6 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_SPI_BFIN_V3) += spi-bfin-v3.o obj-$(CONFIG_SPI_BFIN_SPORT) += spi-bfin-sport.o obj-$(CONFIG_SPI_BITBANG) += spi-bitbang.o obj-$(CONFIG_SPI_BUTTERFLY)+= spi-butterfly.o +obj-$(CONFIG_SPI_CADENCE) += spi-cadence.o obj-$(CONFIG_SPI_CLPS711X) += spi-clps711x.o obj-$(CONFIG_SPI_COLDFIRE_QSPI)+= spi-coldfire-qspi.o obj-$(CONFIG_SPI_DAVINCI) += spi-davinci.o diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c new file mode 100644 index 000..9a8289e --- /dev/null +++ b/drivers/spi/spi-cadence.c @@ -0,0 +1,673 @@ +/* + * Cadence SPI controller driver (master mode only) + * + * Copyright (C) 2008 - 2014 Xilinx, Inc. + * + * based on Blackfin On-Chip SPI Driver (spi_bfin5xx.c) + * + * 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 Found
[PATCH v3 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Add spi-cadence bindings documentation.
Signed-off-by: Harini Katakam
---
v3 changes:
- Remove /bits/ 16 from num-cs property in example.
- Add is-decoded-cs optional property and add to description of num-cs.
v2 changes:
- Separate patch for bindings.
- Add xilinx compatible string; Make compatible string first in the node.
- Use property name num-cs. Make this property optional.
---
.../devicetree/bindings/spi/spi-cadence.txt| 31
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-cadence.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-cadence.txt
b/Documentation/devicetree/bindings/spi/spi-cadence.txt
new file mode 100644
index 000..94f0914
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-cadence.txt
@@ -0,0 +1,31 @@
+Cadence SPI controller Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "cdns,spi-r1p6" or "xlnx,zynq-spi-r1p6".
+- reg : Physical base address and size of SPI registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- clock-names : List of input clock names - "ref_clk", "pclk"
+ (See clock bindings for details).
+- clocks : Clock phandles (see clock bindings for details).
+
+Optional properties:
+- num-cs : Number of chip selects used.
+ If a decoder is used, this will be the number of
+ chip selects after the decoder.
+- is-decoded-cs: Flag to indicate whether decoder is used or
not.
+
+Example:
+
+ spi@e0007000 {
+ compatible = "xlnx,zynq-spi-r1p6";
+ clock-names = "ref_clk", "pclk";
+ clocks = <&clkc 26>, <&clkc 35>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 49 4>;
+ num-cs = <4>;
+ is-decoded-cs = <0>;
+ reg = <0xe0007000 0x1000>;
+ } ;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] SPI: Add driver for Cadence SPI controller
Hi Mark, On Thu, Apr 10, 2014 at 5:51 PM, Mark Brown wrote: > On Thu, Apr 10, 2014 at 05:43:49PM +0530, Harini Katakam wrote: > >> Firstly, the timeout value obtained from this is a too low. >> This timeout is typically used for an error conditions such as >> hardware hang etc. and using a value >1*HZ would be better. >> This driver used to use similar timeout and as I understand, other >> drivers also use similar values. > > Send patches... Ok thanks, I will. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: core: Increase timeout value
The existing timeout value in wait_for_completion_timeout is
calculated from the transfer length and speed with tolerance of 10msec.
This is too low because this is used for error conditions such as
hardware hang etc.
The xfer->speed_hz considered may not be the actual speed set
because the best clock divisor is chosen from a limited set such that
the actual speed <= requested speed. This will lead to timeout being
less than actual transfer time.
Considering acceptable latencies, this timeout can be set to a large
value >= 1*HZ typically.
This patch adds a tolerance of 2000 msec in the core accordingly.
Signed-off-by: Harini Katakam
---
drivers/spi/spi.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4eb9bf0..3fdecfa 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -775,7 +775,7 @@ static int spi_transfer_one_message(struct spi_master
*master,
if (ret > 0) {
ret = 0;
ms = xfer->len * 8 * 1000 / xfer->speed_hz;
- ms += 10; /* some tolerance */
+ ms += 2000; /* some tolerance */
ms =
wait_for_completion_timeout(&master->xfer_completion,
msecs_to_jiffies(ms));
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: core: Increase timeout value
Hi Mark, On Thu, Apr 10, 2014 at 11:06 PM, Mark Brown wrote: > On Thu, Apr 10, 2014 at 06:20:29PM +0530, Harini Katakam wrote: > >> Considering acceptable latencies, this timeout can be set to a large >> value >= 1*HZ typically. > >> This patch adds a tolerance of 2000 msec in the core accordingly. > > That's too much, it's 2 seconds which gets to be incredibly painful when > trying to debug problems - if you're sitting there waiting for a driver > to time out some operations (and it may be more than one of them) so you > can look at the diagnostics it can be quite aggrivating. That's why the > delays are related to the expected runtime for the operation. Something > like double the expected runtime plus something in the 100ms or so range > perhaps? > OK. > Ideally we'd use the actual speed the device set rather than the > requested one too, that'd help. How would you propose to do that - driver should write back actual speed set to xfer->speed_hz? Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] SPI: Add driver for Cadence SPI controller
Hi Mark,
On Fri, Apr 11, 2014 at 4:18 AM, Mark Brown wrote:
> On Thu, Apr 10, 2014 at 05:43:49PM +0530, Harini Katakam wrote:
>> Add driver for Cadence SPI controller. This is used in Xilinx Zynq.
>
> This looks mostly good, the issues below are very small.
>
>> +static int __maybe_unused cdns_spi_suspend(struct device *dev)
>> +{
>> + struct platform_device *pdev = container_of(dev,
>> + struct platform_device, dev);
>> + struct spi_master *master = platform_get_drvdata(pdev);
>> + struct cdns_spi *xspi = spi_master_get_devdata(master);
>> +
>> + spi_master_suspend(master);
>> +
>> + clk_disable(xspi->ref_clk);
>> +
>> + clk_disable(xspi->pclk);
>
> You're only doing clk_disable() here, I would expect the clocks to also
> be unprepared over suspend - there is no value in leaving them prepared
> that I can see. Just use clk_unprepare_disable().
>
> It would also be good (though not essential) to implement runtime PM and
> disable the clocks while there are no transfers in progress for a small
> power saving. The auto_runtime_pm feature in the core will do the
> runtime PM calls for you.
>
> Otherwise this looks good.
Thanks. I'll make these changes and send a v4 next week.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] spi: core: Increase timeout value
The existing timeout value in wait_for_completion_timeout is
calculated from the transfer length and speed with tolerance of 10msec.
This is too low because this is used for error conditions such as
hardware hang etc.
The xfer->speed_hz considered may not be the actual speed set
because the best clock divisor is chosen from a limited set such that
the actual speed <= requested speed. This will lead to timeout being
less than actual transfer time.
Considering acceptable latencies, this timeout can be set to a
value double the expected transfer plus 100 msecs.
This patch adds the same in the core.
Signed-off-by: Harini Katakam
---
v2 changes:
Decrease timeout - make it double of expected time + 100ms
---
drivers/spi/spi.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4eb9bf0..f01cbb4 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -775,7 +775,7 @@ static int spi_transfer_one_message(struct spi_master
*master,
if (ret > 0) {
ret = 0;
ms = xfer->len * 8 * 1000 / xfer->speed_hz;
- ms += 10; /* some tolerance */
+ ms += ms + 100; /* some tolerance */
ms =
wait_for_completion_timeout(&master->xfer_completion,
msecs_to_jiffies(ms));
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] GPIO: Add driver for Zynq GPIO controller
Add support for GPIO controller used by Xilinx Zynq
Signed-off-by: Harini Katakam
---
drivers/gpio/Kconfig |7 +
drivers/gpio/Makefile|1 +
drivers/gpio/gpio-zynq.c | 690 ++
3 files changed, 698 insertions(+)
create mode 100644 drivers/gpio/gpio-zynq.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 903f24d..67a22a6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -313,6 +313,13 @@ config GPIO_XTENSA
Say yes here to support the Xtensa internal GPIO32 IMPWIRE (input)
and EXPSTATE (output) ports
+config GPIO_ZYNQ
+ bool "Xilinx ZYNQ GPIO support"
+ depends on ARCH_ZYNQ
+ select GENERIC_IRQ_CHIP
+ help
+Say yes here to support Xilinx ZYNQ GPIO controller.
+
config GPIO_VR41XX
tristate "NEC VR4100 series General-purpose I/O Uint support"
depends on CPU_VR41XX
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d50179..439f23a 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_GPIO_WM8350) += gpio-wm8350.o
obj-$(CONFIG_GPIO_WM8994) += gpio-wm8994.o
obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o
obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o
+obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o
diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
new file mode 100644
index 000..1f5fdfc
--- /dev/null
+++ b/drivers/gpio/gpio-zynq.c
@@ -0,0 +1,690 @@
+/*
+ * Xilinx Zynq GPIO device driver
+ *
+ * Copyright (C) 2009 - 2014 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
under
+ * the terms of the GNU General Public License as published by the Free
Software
+ * Foundation; either version 2 of the License, or (at your option) any later
+ * version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME "zynq-gpio"
+#define ZYNQ_GPIO_NR_GPIOS 118
+
+static struct irq_domain *irq_domain;
+
+/* Register offsets for the GPIO device */
+
+/* LSW Mask & Data -WO */
+#define ZYNQ_GPIO_DATA_LSW_OFFSET(BANK)(0x000 + (8 * BANK))
+/* MSW Mask & Data -WO */
+#define ZYNQ_GPIO_DATA_MSW_OFFSET(BANK)(0x004 + (8 * BANK))
+/* Data Register-RW */
+#define ZYNQ_GPIO_DATA_OFFSET(BANK)(0x040 + (4 * BANK))
+/* Direction mode reg-RW */
+#define ZYNQ_GPIO_DIRM_OFFSET(BANK)(0x204 + (0x40 * BANK))
+/* Output enable reg-RW */
+#define ZYNQ_GPIO_OUTEN_OFFSET(BANK) (0x208 + (0x40 * BANK))
+/* Interrupt mask reg-RO */
+#define ZYNQ_GPIO_INTMASK_OFFSET(BANK) (0x20C + (0x40 * BANK))
+/* Interrupt enable reg-WO */
+#define ZYNQ_GPIO_INTEN_OFFSET(BANK) (0x210 + (0x40 * BANK))
+/* Interrupt disable reg-WO */
+#define ZYNQ_GPIO_INTDIS_OFFSET(BANK) (0x214 + (0x40 * BANK))
+/* Interrupt status reg-RO */
+#define ZYNQ_GPIO_INTSTS_OFFSET(BANK) (0x218 + (0x40 * BANK))
+/* Interrupt type reg-RW */
+#define ZYNQ_GPIO_INTTYPE_OFFSET(BANK) (0x21C + (0x40 * BANK))
+/* Interrupt polarity reg-RW */
+#define ZYNQ_GPIO_INTPOL_OFFSET(BANK) (0x220 + (0x40 * BANK))
+/* Interrupt on any, reg-RW */
+#define ZYNQ_GPIO_INTANY_OFFSET(BANK) (0x224 + (0x40 * BANK))
+
+/* Read/Write access to the GPIO PS registers */
+static inline u32 zynq_gpio_readreg(void __iomem *offset)
+{
+ return readl_relaxed(offset);
+}
+
+static inline void zynq_gpio_writereg(void __iomem *offset, u32 val)
+{
+ writel_relaxed(val, offset);
+}
+
+static unsigned int zynq_gpio_pin_table[] = {
+ 31, /* 0 - 31 */
+ 53, /* 32 - 53 */
+ 85, /* 54 - 85 */
+ 117 /* 86 - 117 */
+};
+
+/* Maximum banks */
+#define ZYNQ_GPIO_MAX_BANK 4
+
+/* Disable all interrupts mask */
+#define ZYNQ_GPIO_IXR_DISABLE_ALL 0x
+
+/* GPIO pin high */
+#define ZYNQ_GPIO_PIN_HIGH 1
+
+/* Mid pin number of a bank */
+#define ZYNQ_GPIO_MID_PIN_NUM 16
+
+/* GPIO upper 16 bit mask */
+#define ZYNQ_GPIO_UPPER_MASK 0x
+
+/**
+ * struct zynq_gpio - gpio device private data structure
+ * @chip: instance of the gpio_chip
+ * @base_addr: base address of the GPIO device
+ * @irq: irq associated with the controller
+ * @irq_base: base of IRQ number for interrupt
+ * @clk: clock resource for this controller
+ */
+struct zynq_gpio {
+ struct gpio_chip chip;
+ void __iomem *base_addr;
+ unsigned int irq;
+ unsigned int irq_base;
+ struct clk *clk;
+};
+
+/**
+ * zynq_gpio_get_bank_pin - Get the bank number and pin number within that bank
+ * for a given pin in the GPIO device
+ * @pin_num: gpio pin number within the device
+ * @bank_num: an output parameter used to return the bank number of the gpio
+ * pin
+ * @bank_pin_num: an output parameter used to return pin number within a bank
+ * for the given gpio pin
+ *
+ * Returns the
[PATCH 2/2] Devicetree: Add Zynq GPIO devicetree bindings documentation
Add gpio-zynq bindings documentation.
Signed-off-by: Harini Katakam
---
.../devicetree/bindings/gpio/gpio-zynq.txt | 24
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-zynq.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
new file mode 100644
index 000..1b6e83f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
@@ -0,0 +1,24 @@
+Xilinx Zynq GPIO controller Device Tree Bindings
+---
+
+Required properties:
+- #gpio-cells : Should be two. First cell is used to mention
+ pin number.
+- compatible : Should be "xlnx,zynq-gpio-1.0"
+- clocks : Clock phandles (see clock bindings for details)
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- reg : Address and length of the register set for the device
+
+Example:
+ gpio@e000a000 {
+ #gpio-cells = <2>;
+ compatible = "xlnx,zynq-gpio-1.0";
+ clocks = <&clkc 42>;
+ gpio-controller ;
+ interrupt-parent = <&intc>;
+ interrupts = <0 20 4>;
+ reg = <0xe000a000 0x1000>;
+ } ;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4 1/2] SPI: Add driver for Cadence SPI controller
Add driver for Cadence SPI controller. This is used in Xilinx Zynq. Signed-off-by: Harini Katakam --- v4 changes: - Use clk_disable_unprepare and clk_prepare_enable in suspend and resume respectively. v3 changes: - Remove setup function. Make clock CPOL/CPHA setup a separate funtion and call this from prepare_transfer_hardware before enabling. - Remove clk_enable/disable in prepare/unprepare_tranfer_hardware. Clock enable/diable pairs will be retained as before only in probe/remove and suspend/resume. Use of runtime_pms for efficiency will be planned later. - Remove /bits/ in bindings for num-cs, use temporary u32 variable in driver. - Rename requested_bytes and remaining_bytes to tx_bytes/rx_bytes. - Remove unused macros. - Add is-decoded-cs property and add support in driver set_cs funtion. v2 changes: - Use xilinx compatible string too. - Changes read register and write register functions to static inline. - Removed unecessary dev_info and dev_dbg prints. - Return IRQ_HANDLED only when interrupt is handled. - Use a default num-cs value. - Do init_hardware before requesting irq. - Remove unecessary master_put() - Set master->max_speed_hz - Check for busy in cdns_setup(). Retained this function with this check as opposed to removing. The reason for this is clock configuration needs to be done for the first time before enable is done in prepare_hardware; prepare_hardware however, doesn't receive spi_device structure. - Implememnt transfer_one instead of transfer_one_message. Remove wait_for_completion as this is done by core. - Implement set_cs. - Clock enable/disable in prepare/unprepare respectively. - In suspend, remove reset of hardware; simply call unprepare_hardware. - In suspend/resume call master_suspend/resume respectively. - Check for irq<=0 in probe. - Remove MODULE_ALIAS. --- drivers/spi/Kconfig |7 + drivers/spi/Makefile |1 + drivers/spi/spi-cadence.c | 673 + 3 files changed, 681 insertions(+) create mode 100644 drivers/spi/spi-cadence.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index efe1960..b0f091b 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -148,6 +148,13 @@ config SPI_BUTTERFLY inexpensive battery powered microcontroller evaluation board. This same cable can be used to flash new firmware. +config SPI_CADENCE + tristate "Cadence SPI controller" + depends on SPI_MASTER + help + This selects the Cadence SPI controller master driver + used by Xilinx Zynq. + config SPI_CLPS711X tristate "CLPS711X host SPI controller" depends on ARCH_CLPS711X || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index bd79266..3e503d6 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_SPI_BFIN_V3) += spi-bfin-v3.o obj-$(CONFIG_SPI_BFIN_SPORT) += spi-bfin-sport.o obj-$(CONFIG_SPI_BITBANG) += spi-bitbang.o obj-$(CONFIG_SPI_BUTTERFLY)+= spi-butterfly.o +obj-$(CONFIG_SPI_CADENCE) += spi-cadence.o obj-$(CONFIG_SPI_CLPS711X) += spi-clps711x.o obj-$(CONFIG_SPI_COLDFIRE_QSPI)+= spi-coldfire-qspi.o obj-$(CONFIG_SPI_DAVINCI) += spi-davinci.o diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c new file mode 100644 index 000..bb75897 --- /dev/null +++ b/drivers/spi/spi-cadence.c @@ -0,0 +1,673 @@ +/* + * Cadence SPI controller driver (master mode only) + * + * Copyright (C) 2008 - 2014 Xilinx, Inc. + * + * based on Blackfin On-Chip SPI Driver (spi_bfin5xx.c) + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License version 2 as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Name of this driver */ +#define CDNS_SPI_NAME "cdns-spi" + +/* Register offset definitions */ +#define CDNS_SPI_CR_OFFSET 0x00 /* Configuration Register, RW */ +#define CDNS_SPI_ISR_OFFSET0x04 /* Interrupt Status Register, RO */ +#define CDNS_SPI_IER_OFFSET0x08 /* Interrupt Enable Register, WO */ +#define CDNS_SPI_IDR_OFFSET0x0c /* Interrupt Disable Register, WO */ +#define CDNS_SPI_IMR_OFFSET0x10 /* Interrupt Enabled Mask Register, RO */ +#define CDNS_SPI_ER_OFFSET 0x14 /* Enable/Disable Register, RW */ +#define CDNS_SPI_DR_OFFSET 0x18 /* Delay Register, RW */ +#define CDNS_SPI_TXD_OFFSET0x1C /* Data Transmit Register, WO */ +#define CDNS_SPI_RXD_OFFSET0x20 /* Data Receive Register, RO */ +#define CDNS_SPI_SICR_OFFSET 0x24 /* Slave Idle Count Register, RW */ +#define CDNS_SPI_THLD_OFFSET 0x28 /* Transmit FIFO Watermark R
[PATCH v4 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI
Add spi-cadence bindings documentation.
Signed-off-by: Harini Katakam
---
v4 changes:
No changes.
v3 changes:
- Remove /bits/ 16 from num-cs property in example.
- Add is-decoded-cs optional property and add to description of num-cs.
v2 changes:
- Separate patch for bindings.
- Add xilinx compatible string; Make compatible string first in the node.
- Use property name num-cs. Make this property optional.
---
.../devicetree/bindings/spi/spi-cadence.txt| 31
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-cadence.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-cadence.txt
b/Documentation/devicetree/bindings/spi/spi-cadence.txt
new file mode 100644
index 000..94f0914
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-cadence.txt
@@ -0,0 +1,31 @@
+Cadence SPI controller Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "cdns,spi-r1p6" or "xlnx,zynq-spi-r1p6".
+- reg : Physical base address and size of SPI registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- clock-names : List of input clock names - "ref_clk", "pclk"
+ (See clock bindings for details).
+- clocks : Clock phandles (see clock bindings for details).
+
+Optional properties:
+- num-cs : Number of chip selects used.
+ If a decoder is used, this will be the number of
+ chip selects after the decoder.
+- is-decoded-cs: Flag to indicate whether decoder is used or
not.
+
+Example:
+
+ spi@e0007000 {
+ compatible = "xlnx,zynq-spi-r1p6";
+ clock-names = "ref_clk", "pclk";
+ clocks = <&clkc 26>, <&clkc 35>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 49 4>;
+ num-cs = <4>;
+ is-decoded-cs = <0>;
+ reg = <0xe0007000 0x1000>;
+ } ;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] SPI: Add driver for Cadence SPI controller
Hi Mark,
On Fri, Apr 11, 2014 at 9:06 AM, Harini Katakam
wrote:
> Hi Mark,
>
> On Fri, Apr 11, 2014 at 4:18 AM, Mark Brown wrote:
>> On Thu, Apr 10, 2014 at 05:43:49PM +0530, Harini Katakam wrote:
>>> Add driver for Cadence SPI controller. This is used in Xilinx Zynq.
>>
>> This looks mostly good, the issues below are very small.
>>
>>> +static int __maybe_unused cdns_spi_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = container_of(dev,
>>> + struct platform_device, dev);
>>> + struct spi_master *master = platform_get_drvdata(pdev);
>>> + struct cdns_spi *xspi = spi_master_get_devdata(master);
>>> +
>>> + spi_master_suspend(master);
>>> +
>>> + clk_disable(xspi->ref_clk);
>>> +
>>> + clk_disable(xspi->pclk);
>>
>> You're only doing clk_disable() here, I would expect the clocks to also
>> be unprepared over suspend - there is no value in leaving them prepared
>> that I can see. Just use clk_unprepare_disable().
>>
>> It would also be good (though not essential) to implement runtime PM and
>> disable the clocks while there are no transfers in progress for a small
>> power saving. The auto_runtime_pm feature in the core will do the
>> runtime PM calls for you.
>>
>> Otherwise this looks good.
>
> Thanks. I'll make these changes and send a v4 next week.
>
This driver without runtime PM will suffice our current requirements.
Disabling the clocks will offer some small power savings as you pointed out and
I will revisit this at a later time as an enhancement.
I have addressed the other review comment and sent out a v4.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] Add support for flag status register on Micron chips.
Hi,
On Fri, Apr 11, 2014 at 8:33 PM, wrote:
> From: Graham Moore
>
> Some new Micron flash chips require reading the flag
> status register to determine when operations have completed.
>
> Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> require reading the status register before reading the flag status register.
>
> This patch adds support for the flag status register in the n25q512a1 and
> n25q00
> Micron QSPI flash chips.
>
> Signed-off-by: Graham Moore
> #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
> @@ -941,6 +999,8 @@ static const struct spi_device_id m25p_ids[] = {
> { "n25q128a13", INFO(0x20ba18, 0, 64 * 1024, 256, 0) },
> { "n25q256a",INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) },
> { "n25q512a",INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) },
> + { "n25q512a1", INFO(0x20ba20, 0, 64 * 1024, 1024, USE_FSR) },
> + { "n25q00", INFO(0x20ba21, 0, 64 * 1024, 2048, USE_FSR) },
>
I understand that "n25q512a1" was added to distinguish between
0x20bb20 and 0x20ba20,
which is essentially 1.8V and 3V parts.
(The actual part numbers are n25q512a11 and n25q512a13 respectively)
But USE_FSR is required for both parts.
Sorry for posting this question here but it seemed relevant:
When such devices differ only in supply voltages (and return different
response to READ ID),
which we don't act on, is there a way to use the same string?
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 1/2] SPI: Add driver for Cadence SPI controller
Hi Mark, On Tue, Apr 15, 2014 at 1:31 AM, Mark Brown wrote: > On Mon, Apr 14, 2014 at 02:36:53PM +0530, Harini Katakam wrote: >> Add driver for Cadence SPI controller. This is used in Xilinx Zynq. > > Applied both, thanks. Please use subject lines consistent with the > style for the subsystem. Thanks! I'll keep that in mind. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: linux-next: build failure after merge of the spi tree
Hi Stephen/Mark, > -Original Message- > From: Stephen Rothwell [mailto:s...@canb.auug.org.au] > Sent: Tuesday, April 15, 2014 7:47 AM > To: Mark Brown > Cc: linux-n...@vger.kernel.org; linux-kernel@vger.kernel.org; Harini > Katakam > Subject: linux-next: build failure after merge of the spi tree > > Hi Mark, > > After merging the spi tree, today's linux-next build (x86_64_allmodconfig) > failed like this: > > drivers/spi/spi-cadence.c: In function 'cdns_spi_write': > drivers/spi/spi-cadence.c:135:2: error: implicit declaration of function > 'writel_relaxed' [-Werror=implicit-function-declaration] > writel_relaxed(val, xspi->regs + offset); > ^ > I'm sorry - I just noticed that Soren's patches for I2C also gave a similar warning: http://patchwork.ozlabs.org/patch/337285/ I will send a patch adding dependency on ARCH_ZYNQ. Please let me know if you have a better suggestion. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: cadence: Add dependency on ARCH_ZYNQ
Add dependency on ARCH_ZYNQ in Kconfig. This is to fix the build error. Signed-off-by: Harini Katakam --- drivers/spi/Kconfig |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index b0f091b..0db219b 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -150,6 +150,7 @@ config SPI_BUTTERFLY config SPI_CADENCE tristate "Cadence SPI controller" + depends on ARCH_ZYNQ depends on SPI_MASTER help This selects the Cadence SPI controller master driver -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] spi: cadence: Add dependency on ARCH_ARM
Add dependency on ARCH_ARM in Kconfig. This is to fix the build error related to _relaxed IO. Signed-off-by: Harini Katakam --- v2 changes: Dependency on ARCH_ARM instead of restricting to ARCH_ZYNQ. --- drivers/spi/Kconfig |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index b0f091b..32c27dd 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -150,6 +150,7 @@ config SPI_BUTTERFLY config SPI_CADENCE tristate "Cadence SPI controller" + depends on ARCH_ARM depends on SPI_MASTER help This selects the Cadence SPI controller master driver -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] spi: cadence: Add dependency on ARCH_ARM
Hi, On Tue, Apr 15, 2014 at 3:52 PM, Paul Bolle wrote: > On Tue, 2014-04-15 at 15:42 +0530, Harini Katakam wrote: >> Add dependency on ARCH_ARM in Kconfig. >> This is to fix the build error related to _relaxed IO. >> >> Signed-off-by: Harini Katakam >> --- >> >> v2 changes: >> Dependency on ARCH_ARM instead of restricting to ARCH_ZYNQ. >> >> --- >> drivers/spi/Kconfig |1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >> index b0f091b..32c27dd 100644 >> --- a/drivers/spi/Kconfig >> +++ b/drivers/spi/Kconfig >> @@ -150,6 +150,7 @@ config SPI_BUTTERFLY >> >> config SPI_CADENCE >> tristate "Cadence SPI controller" >> + depends on ARCH_ARM > > Did you mean just ARM? Please ignore patch, I will resend it. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] spi: cadence: Add dependency on ARM
Add dependency on ARM in Kconfig. This is to fix the build error related to _relaxed IO. Remove dependency on SPI_MASTER because this is already defined under if SPI_MASTER in Kconfig. Signed-off-by: Harini Katakam --- v2 changes: Add dependency on ARM instead of ZYNQ. Remove SPI_MASTER dependency as it is already under if SPI_MASTER. --- drivers/spi/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index b0f091b..0616f97 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -150,7 +150,7 @@ config SPI_BUTTERFLY config SPI_CADENCE tristate "Cadence SPI controller" - depends on SPI_MASTER + depends on ARM help This selects the Cadence SPI controller master driver used by Xilinx Zynq. -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] devicetree: Add Zynq GPIO devicetree bindings documentation
From: Harini Katakam
Add gpio-zynq bindings documentation.
Signed-off-by: Harini Katakam
Signed-off-by: Soren Brinkmann
---
v2 changes:
Improve description.
---
.../devicetree/bindings/gpio/gpio-zynq.txt | 24
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-zynq.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
new file mode 100644
index 000..b2c023a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
@@ -0,0 +1,24 @@
+Xilinx Zynq GPIO controller Device Tree Bindings
+---
+
+Required properties:
+- #gpio-cells : Should be two. First cell is used to mention
+ pin number.
+- compatible : Should be "xlnx,zynq-gpio-1.0"
+- clocks : Clock specifier (see clock bindings for details)
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+- interrupt-parent : Must be core interrupt controller
+- reg : Address and length of the register set for the device
+
+Example:
+ gpio@e000a000 {
+ #gpio-cells = <2>;
+ compatible = "xlnx,zynq-gpio-1.0";
+ clocks = <&clkc 42>;
+ gpio-controller;
+ interrupt-parent = <&intc>;
+ interrupts = <0 20 4>;
+ reg = <0xe000a000 0x1000>;
+ };
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] gpio: Add driver for Zynq GPIO controller
From: Harini Katakam Add support for GPIO controller used by Xilinx Zynq. Signed-off-by: Harini Katakam Signed-off-by: Soren Brinkmann --- v2 changes: - convert to pm_runtime_force_(suspend|resume) - add pm_runtime_set_active in probe() - also (un)prepare clocks when they are dis-/enabled - add some missing calls to pm_runtime_get() - use pm_runtime_put() instead of sync variant - remove gpio chip in driver remove() - remove redundant type casts - directly use IO helpers - use BIT macro to set/clear bits - migrate to GPIOLIB_IRQCHIP --- drivers/gpio/Kconfig |7 + drivers/gpio/Makefile|1 + drivers/gpio/gpio-zynq.c | 651 ++ 3 files changed, 659 insertions(+) create mode 100644 drivers/gpio/gpio-zynq.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 4a1b511..bdeef02 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -340,6 +340,13 @@ config GPIO_XILINX help Say yes here to support the Xilinx FPGA GPIO device +config GPIO_ZYNQ + tristate "Xilinx Zynq GPIO support" + depends on ARCH_ZYNQ + select GPIOLIB_IRQCHIP + help + Say yes here to support Xilinx Zynq GPIO controller. + config GPIO_XTENSA bool "Xtensa GPIO32 support" depends on XTENSA diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index d10f6a9..033fd7c 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -101,3 +101,4 @@ obj-$(CONFIG_GPIO_WM8994) += gpio-wm8994.o obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o obj-$(CONFIG_GPIO_ZEVIO) += gpio-zevio.o +obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c new file mode 100644 index 000..3af13b6 --- /dev/null +++ b/drivers/gpio/gpio-zynq.c @@ -0,0 +1,651 @@ +/* + * Xilinx Zynq GPIO device driver + * + * Copyright (C) 2009 - 2014 Xilinx, Inc. + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 2 of the License, or (at your option) any later + * version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_NAME "zynq-gpio" + +/* Maximum banks */ +#define ZYNQ_GPIO_MAX_BANK 4 + +#define ZYNQ_GPIO_BANK0_NGPIO 32 +#define ZYNQ_GPIO_BANK1_NGPIO 22 +#define ZYNQ_GPIO_BANK2_NGPIO 32 +#define ZYNQ_GPIO_BANK3_NGPIO 32 + +#define ZYNQ_GPIO_NR_GPIOS (ZYNQ_GPIO_BANK0_NGPIO + \ +ZYNQ_GPIO_BANK1_NGPIO + \ +ZYNQ_GPIO_BANK2_NGPIO + \ +ZYNQ_GPIO_BANK3_NGPIO) + +#define ZYNQ_GPIO_BANK0_PIN_MIN0 +#define ZYNQ_GPIO_BANK0_PIN_MAX(ZYNQ_GPIO_BANK0_PIN_MIN + \ + ZYNQ_GPIO_BANK0_NGPIO - 1) +#define ZYNQ_GPIO_BANK1_PIN_MIN(ZYNQ_GPIO_BANK0_PIN_MAX + 1) +#define ZYNQ_GPIO_BANK1_PIN_MAX(ZYNQ_GPIO_BANK1_PIN_MIN + \ + ZYNQ_GPIO_BANK1_NGPIO - 1) +#define ZYNQ_GPIO_BANK2_PIN_MIN(ZYNQ_GPIO_BANK1_PIN_MAX + 1) +#define ZYNQ_GPIO_BANK2_PIN_MAX(ZYNQ_GPIO_BANK2_PIN_MIN + \ + ZYNQ_GPIO_BANK2_NGPIO - 1) +#define ZYNQ_GPIO_BANK3_PIN_MIN(ZYNQ_GPIO_BANK2_PIN_MAX + 1) +#define ZYNQ_GPIO_BANK3_PIN_MAX(ZYNQ_GPIO_BANK3_PIN_MIN + \ + ZYNQ_GPIO_BANK3_NGPIO - 1) + + +/* Register offsets for the GPIO device */ +/* LSW Mask & Data -WO */ +#define ZYNQ_GPIO_DATA_LSW_OFFSET(BANK)(0x000 + (8 * BANK)) +/* MSW Mask & Data -WO */ +#define ZYNQ_GPIO_DATA_MSW_OFFSET(BANK)(0x004 + (8 * BANK)) +/* Data Register-RW */ +#define ZYNQ_GPIO_DATA_RO_OFFSET(BANK) (0x060 + (4 * BANK)) +/* Direction mode reg-RW */ +#define ZYNQ_GPIO_DIRM_OFFSET(BANK)(0x204 + (0x40 * BANK)) +/* Output enable reg-RW */ +#define ZYNQ_GPIO_OUTEN_OFFSET(BANK) (0x208 + (0x40 * BANK)) +/* Interrupt mask reg-RO */ +#define ZYNQ_GPIO_INTMASK_OFFSET(BANK) (0x20C + (0x40 * BANK)) +/* Interrupt enable reg-WO */ +#define ZYNQ_GPIO_INTEN_OFFSET(BANK) (0x210 + (0x40 * BANK)) +/* Interrupt disable reg-WO */ +#define ZYNQ_GPIO_INTDIS_OFFSET(BANK) (0x214 + (0x40 * BANK)) +/* Interrupt status reg-RO */ +#define ZYNQ_GPIO_INTSTS_OFFSET(BANK) (0x218 + (0x40 * BANK)) +/* Interrupt type reg-RW */ +#define ZYNQ_GPIO_INTTYPE_OFFSET(BANK) (0x21C + (0x40 * BANK)) +/* Interrupt polarity reg-RW */ +#define ZYNQ_GPIO_INTPOL_OFFSET(BANK) (0x220 + (0x40 * BANK)) +/* Interrupt on any, reg-RW */ +#define ZYNQ_GPIO_INTANY_OFFSET(BANK) (0x224 + (0x40 * BANK)) + +/* Disable all interrupts mask */ +#define ZYNQ_GPIO_IXR_DISABLE_ALL 0x + +/* Mid pin number of a bank */ +#define ZYNQ_GPIO_MID_P
RE: [PATCH v2 05/11] ARM: dts: zynq: Add QSPI for Parallella
Hi,
> -Original Message-
> From: Michal Simek [mailto:michal.si...@xilinx.com]
> Sent: Friday, July 25, 2014 3:08 PM
> To: Andreas Färber; mon...@monstr.eu; Soren Brinkmann
> Cc: Harini Katakam; Michal Simek; Andreas Olofsson; Matteo Vit; Sean
> Rickerd; devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Mark Rutland; Ian
> Campbell; Kumar Gala; Russell King
> Subject: Re: [PATCH v2 05/11] ARM: dts: zynq: Add QSPI for Parallella
>
> On 07/25/2014 10:42 AM, Andreas Färber wrote:
> > Am 25.07.2014 09:59, schrieb Michal Simek:
> >> On 07/25/2014 01:18 AM, Sören Brinkmann wrote:
> >>> On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
> >>>> Prepare SPI0 and SPI1 while at it.
> >
> >> Patch subject is incorrect. You are adding SPI and QSPI.
> >
> > Yes, it originally added only QSPI, but I considered it a good deed to
> > add SPI as well while already reading that part of the TRM. :)
> >
> >>>>
> >>>> Signed-off-by: Andreas Färber --- v2: New
> >>>>
> >>>> arch/arm/boot/dts/zynq-7000.dtsi | 37
> >>>> +++
> >>>> arch/arm/boot/dts/zynq-parallella.dts | 4 2 files
> >>>> changed, 41 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi
> >>>> b/arch/arm/boot/dts/zynq-7000.dtsi index 8fd826a..eed3df0
> >>>> 100644 --- a/arch/arm/boot/dts/zynq-7000.dtsi +++
> >>>> b/arch/arm/boot/dts/zynq-7000.dtsi @@ -122,6 +122,30 @@
> >>>> interrupts = <0 50 4>; };
> >>>>
> >>>> +spi0: spi@e0006000 { + compatible =
> "xlnx,zynq-spi-r1p6";
> >>>> +reg = <0xe0006000 0x1000>; +
> >>>> status
> = "disabled"; +
> >>>> interrupt-parent = <&intc>; +interrupts = <0 26 4>;
> +
> >>>> clocks = <&clkc 25>, <&clkc 34>; + clock-names =
> "ref_clk",
> >>>> "pclk"; +#address-cells = <1>; +
> >>>> #size-
> cells = <0>; +};
> >>>> + + spi1: spi@e0007000 { + compatible =
> >>>> "xlnx,zynq-spi-r1p6"; + reg = <0xe0007000 0x1000>; +
> status
> >>>> = "disabled"; + interrupt-parent = <&intc>; +
> interrupts =
> >>>> <0 49 4>; + clocks = <&clkc 26>, <&clkc 35>; +
> clock-names
> >>>> = "ref_clk", "pclk"; + #address-cells = <1>; +
> #size-cells
> >>>> = <0>; + }; +
> >>> Until here things look good.
> >>>
> >>>> gem0: ethernet@e000b000 { compatible = "cdns,gem"; reg =
> >>>> <0xe000b000 0x4000>; @@ -140,6 +164,19 @@ clock-names = "pclk",
> >>>> "hclk", "tx_clk"; };
> >>>>
> >>>> +qspi: qspi@e000d000 { + compatible =
> >>>> "xlnx,zynq-spi-r1p6"; + reg = <0xe000d000 0x1000>; +
> status
> >>>> = "disabled"; + interrupt-parent = <&intc>; +
> interrupts =
> >>>> <0 19 4>; + clocks = <&clkc 10>, <&clkc 43>; +
> clock-names
> >>>> = "ref_clk", "pclk"; + num-cs = <1>; +
> #address-cells =
> >>>> <1>; + #size-cells = <0>; +}; +
> >>> I'm not sure what the status of this driver is. I think QSPI is
> >>> still under review on the mailing lists and I don't think we
> >>> should add this yet.
> >
> >> Driver for qspi is not in the mainline yet but it doesn't mean that
> >> this fragment won't work. Harini: Can you please correct me if I am
> >> wrong?
> >
It can be added but it will have to be disabled as there is no qspi driver
at the moment in mainline.
Regards,
Harini
RE: [PATCH v2 05/11] ARM: dts: zynq: Add QSPI for Parallella
Hi,
> -Original Message-
> From: Harini Katakam
> Sent: Friday, July 25, 2014 4:01 PM
> To: 'Michal Simek'; Andreas Färber; mon...@monstr.eu; Soren Brinkmann
> Cc: Michal Simek; Andreas Olofsson; Matteo Vit; Sean Rickerd;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; Rob Herring; Pawel Moll; Mark Rutland; Ian
> Campbell; Kumar Gala; Russell King
> Subject: RE: [PATCH v2 05/11] ARM: dts: zynq: Add QSPI for Parallella
>
> Hi,
>
> > -Original Message-
> > From: Michal Simek [mailto:michal.si...@xilinx.com]
> > Sent: Friday, July 25, 2014 3:08 PM
> > To: Andreas Färber; mon...@monstr.eu; Soren Brinkmann
> > Cc: Harini Katakam; Michal Simek; Andreas Olofsson; Matteo Vit; Sean
> > Rickerd; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org;
> > linux-kernel@vger.kernel.org; Rob Herring; Pawel Moll; Mark Rutland; Ian
> > Campbell; Kumar Gala; Russell King
> > Subject: Re: [PATCH v2 05/11] ARM: dts: zynq: Add QSPI for Parallella
> >
> > On 07/25/2014 10:42 AM, Andreas Färber wrote:
> > > Am 25.07.2014 09:59, schrieb Michal Simek:
> > >> On 07/25/2014 01:18 AM, Sören Brinkmann wrote:
> > >>> On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
> > >>>> Prepare SPI0 and SPI1 while at it.
> > >
> > >> Patch subject is incorrect. You are adding SPI and QSPI.
> > >
> > > Yes, it originally added only QSPI, but I considered it a good deed to
> > > add SPI as well while already reading that part of the TRM. :)
> > >
> > >>>>
> > >>>> Signed-off-by: Andreas Färber --- v2: New
> > >>>>
> > >>>> arch/arm/boot/dts/zynq-7000.dtsi | 37
> > >>>> +++
> > >>>> arch/arm/boot/dts/zynq-parallella.dts | 4 2 files
> > >>>> changed, 41 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi
> > >>>> b/arch/arm/boot/dts/zynq-7000.dtsi index 8fd826a..eed3df0
> > >>>> 100644 --- a/arch/arm/boot/dts/zynq-7000.dtsi +++
> > >>>> b/arch/arm/boot/dts/zynq-7000.dtsi @@ -122,6 +122,30 @@
> > >>>> interrupts = <0 50 4>; };
> > >>>>
> > >>>> + spi0: spi@e0006000 { + compatible =
> > "xlnx,zynq-spi-r1p6";
> > >>>> + reg = <0xe0006000 0x1000>; +
> status
> > = "disabled"; +
> > >>>> interrupt-parent = <&intc>; + interrupts = <0 26 4>;
> > +
> > >>>> clocks = <&clkc 25>, <&clkc 34>; + clock-names
> =
> > "ref_clk",
> > >>>> "pclk"; + #address-cells = <1>; +
> > >>>> #size-
> > cells = <0>; + };
> > >>>> + +spi1: spi@e0007000 { + compatible =
> > >>>> "xlnx,zynq-spi-r1p6"; +reg = <0xe0007000
> > >>>> 0x1000>; +
> > status
> > >>>> = "disabled"; +interrupt-parent = <&intc>; +
> > interrupts =
> > >>>> <0 49 4>; +clocks = <&clkc 26>, <&clkc 35>; +
> > clock-names
> > >>>> = "ref_clk", "pclk"; + #address-cells = <1>; +
> > #size-cells
> > >>>> = <0>; + }; +
> > >>> Until here things look good.
> > >>>
> > >>>> gem0: ethernet@e000b000 { compatible = "cdns,gem"; reg =
> > >>>> <0xe000b000 0x4000>; @@ -140,6 +164,19 @@ clock-names = "pclk",
> > >>>> "hclk", "tx_clk"; };
> > >>>>
> > >>>> + qspi: qspi@e000d000 { +
> compatible =
> > >>>> "xlnx,zynq-spi-r1p6"; +reg = <0xe000d000
> > >>>> 0x1000>; +
> > status
> > >>>> = "disabled"; +interrupt-parent = <&intc>; +
> > interrupts =
> > >>>> <0 19 4>; +clocks = <&clkc 10>, <&clkc 43>; +
> > clock-names
> > >>>> = "ref_clk", "pclk"; + num-cs = <1>; +
> > #address-cells =
> > >>>> <1>; + #size-cells = <0>; +}; +
> > >>> I'm not sure what the status of this driver is. I think QSPI is
> > >>> still under review on the mailing lists and I don't think we
> > >>> should add this yet.
> > >
> > >> Driver for qspi is not in the mainline yet but it doesn't mean that
> > >> this fragment won't work. Harini: Can you please correct me if I am
> > >> wrong?
> > >
>
> It can be added but it will have to be disabled as there is no qspi driver
> at the moment in mainline.
>
The cadence spi driver can't be used for qspi directly.
It’s better not to add qspi now. Once qspi driver is in mainline, qspi
can be added with the corresponding compatibility string.
Regards,
Harini
RE: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
Hi Mark, My mail seems to have missed the below reply. Anyway, please find my answer below: > -Original Message- > From: Punnaiah Choudary Kalluri > Sent: Monday, July 14, 2014 12:03 PM > To: Harini Katakam > Subject: FW: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller > > > > >-Original Message- > >From: Mark Brown [mailto:broo...@kernel.org] > >Sent: Thursday, July 10, 2014 8:37 PM > >To: Harini Katakam > >Cc: Geert Uytterhoeven; Grant Likely; Rob Herring; Pawel Moll; Mark > Rutland; > >Ian Campbell; Kumar Gala; linux-spi; linux-kernel@vger.kernel.org; > >devicet...@vger.kernel.org; linux-...@vger.kernel.org; David > Woodhouse; > >Brian Norris; Marek Vašut; Artem Bityutskiy; Geert Uytterhoeven; Sascha > >Hauer; Jingoo Han; Sourav Poddar; Michal Simek; Punnaiah Choudary > Kalluri > >Subject: Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller > > > >On Thu, Jul 10, 2014 at 06:09:55PM +0530, Harini Katakam wrote: > >> On Thu, Jul 10, 2014 at 5:31 PM, Mark Brown > wrote: > > > >> > How does the client driver select the width to use for a transfer? > > > >> This controller is meant to be used only with flash devices. > >> The flash devices' supported width will be reflected in a table in MTD > layer. > >> When selecting, priority is given to quad over dual and single in the MTD > and > >> it will send commands using the supported tx/rx bus width accordingly. > >> About the supported bus width on board, tx-bus-width and rx-bus-width > >> properties in dts will have the info; which I believe spi core uses. > > > >If it's only intended to be used as a flash controller (and might > >misbehave if used as such, if the command detection false triggers) then > >it is probably better to support it as a flash controller rather than as > >a SPI controller. Or can the flash-specific features be disabled? There is provision to switch to legacy (generic spi) mode but it is not usually used. As you can can see, the flash related specifics come into play only when two flash devices are used. For a single slave, it will be generic. As per your suggestions, I could send this driver in multiple stages - Initially, qspi driver without flash specifics (this will work straight-away for a single flash) In the next set, flash specifics changes for dual flash configurations (parallel/stacked) Is that acceptable? Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
Hi Mark,
On Fri, Jul 11, 2014 at 7:08 PM, Mark Brown wrote:
> On Thu, Jul 10, 2014 at 02:20:06PM +0530, Harini Katakam wrote:
>
>> This patch adds support for QSPI controller used by Zynq.
>
> The driver looks pretty clean but there are a couple of issues below,
> including a little bit more of the flash specifics.
>
>> +static void zynq_qspi_chipselect(struct spi_device *qspi, bool is_high)
>> +{
>> + struct zynq_qspi *xqspi = spi_master_get_devdata(qspi->master);
>> + u32 config_reg;
>> +
>> + config_reg = zynq_qspi_read(xqspi, ZYNQ_QSPI_CONFIG_OFFSET);
>> +
>> + /* Select upper/lower page before asserting CS */
>> + if (xqspi->is_stacked) {
>> + u32 lqspi_cfg_reg;
>
> Like with the dual and quad mode stuff this looks very much like it's
> specific to flash rather than something that applies to a generic SPI
> driver. However it does look like it's a generic SPI device which could
> be used in other applications which makes things a bit tricky. We don't
> have a really good answer for this right now unfortunately, probably we
> need some sort of special interface between the SPI and flash subsystems
> to allow flash to use the flash specific stuff.
>
> For use as a generic SPI device what I'd suggest is stripping out the
> flash specifics, merging the rest of the support and then considering
> the flash specifics separately.
Reply in the other thread.
>
>> +/**
>> + * zynq_prepare_transfer_hardware - Prepares hardware for transfer.
>> + * @master: Pointer to the spi_master structure which provides
>> + * information about the controller.
>> + *
>> + * This function enables SPI master controller.
>> + *
>> + * Return: Always 0
>> + */
>> +static int zynq_prepare_transfer_hardware(struct spi_master *master)
>> +{
>> + struct zynq_qspi *xqspi = spi_master_get_devdata(master);
>> +
>> + zynq_qspi_config_clock_mode(master->cur_msg->spi);
>
> The clock mode needs to be (and is) configured per transfer so I'd
> expect it's possible to remove this call.
>
Yeah I understand. It can go away from here and there should be a
prepare_message as per Lars-Peter's patches on spi-cadence.
I'll reflect the same in this driver.
Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] watchdog: Add Cadence WDT driver
Add Cadence WDT driver. This is used by Xilinx Zynq.
Signed-off-by: Harini Katakam
---
v3 changes:
- Make rst a boolean and use of_property_read_boolean.
- Use only cdns compatible string. If customization is required in the
future, xlnx compatible string can be added.
v2 changes:
- Update IO helpers.
- Use dev_dbg instead of dev_info where possible.
- Use watchdog_init_timeout
- Spinlock for restart register where missing.
- Change order of probe to move reboot notifier register to the end
- Remove unecessary comments
- Do clk_prepare_enabel and clk_disable_unprepare in resume/suspend
respectively.
- There is an input from dts to decide whether internal reset should be
enabled or not. When this is enabled, reset happens wutomatically when
counter reaches zero. In case this is not enabled, a message is disaplayed
to indicate that the watchdog has timed out. Elaborated this message
to describe the above.
---
drivers/watchdog/Kconfig |7 +
drivers/watchdog/Makefile |1 +
drivers/watchdog/cadence_wdt.c | 516
3 files changed, 524 insertions(+)
create mode 100644 drivers/watchdog/cadence_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 76dd541..adf5110 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -147,6 +147,13 @@ config AT91SAM9X_WATCHDOG
Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will
reboot your system when the timeout is reached.
+config CADENCE_WATCHDOG
+ tristate "Cadence Watchdog Timer"
+ select WATCHDOG_CORE
+ help
+ Say Y here if you want to include support for the watchdog
+ timer in the Xilinx Zynq.
+
config 21285_WATCHDOG
tristate "DC21285 watchdog"
depends on FOOTBRIDGE
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 468c320..bae2fb0 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
+obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o
obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
obj-$(CONFIG_TWL4030_WATCHDOG) += twl4030_wdt.o
obj-$(CONFIG_21285_WATCHDOG) += wdt285.o
diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
new file mode 100644
index 000..ce96e20
--- /dev/null
+++ b/drivers/watchdog/cadence_wdt.c
@@ -0,0 +1,516 @@
+/*
+ * Cadence WDT driver - Used by Xilinx Zynq
+ *
+ * Copyright (C) 2010 - 2014 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define CDNS_WDT_DEFAULT_TIMEOUT 10
+/* Supports 1 - 516 sec */
+#define CDNS_WDT_MIN_TIMEOUT 1
+#define CDNS_WDT_MAX_TIMEOUT 516
+
+/* Restart key */
+#define CDNS_WDT_RESTART_KEY 0x1999
+
+/* Counter register access key */
+#define CDNS_WDT_REGISTER_ACCESS_KEY 0x0092
+
+/* Counter value divisor */
+#define CDNS_WDT_COUNTER_VALUE_DIVISOR 0x1000
+
+/* Clock prescaler value and selection */
+#define CDNS_WDT_PRESCALE_64 64
+#define CDNS_WDT_PRESCALE_512 512
+#define CDNS_WDT_PRESCALE_4096 4096
+#define CDNS_WDT_PRESCALE_SELECT_641
+#define CDNS_WDT_PRESCALE_SELECT_512 2
+#define CDNS_WDT_PRESCALE_SELECT_4096 3
+
+/* Input clock frequency */
+#define CDNS_WDT_CLK_10MHZ 1000
+#define CDNS_WDT_CLK_75MHZ 7500
+
+/* Counter maximum value */
+#define CDNS_WDT_COUNTER_MAX 0xFFF
+
+static int wdt_timeout = CDNS_WDT_DEFAULT_TIMEOUT;
+static int nowayout = WATCHDOG_NOWAYOUT;
+
+module_param(wdt_timeout, int, 0);
+MODULE_PARM_DESC(wdt_timeout,
+"Watchdog time in seconds. (default="
+__MODULE_STRING(CDNS_WDT_DEFAULT_TIMEOUT) ")");
+
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout,
+"Watchdog cannot be stopped once started (default="
+__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+/**
+ * struct cdns_wdt - Watchdog device structure
+ * @regs: baseaddress of device
+ * @rst: reset flag
+ * @clk: struct clk * of a clock source
+ * @prescaler: for saving prescaler value
+ * @ctrl_clksel: counter clock prescaler selection
+ * @io_lock: spinlock for IO register access
+ * @cdns_wdt_device: watchdog device structure
+ * @cdns_wdt_notifier: notifier structure
+ *
+ * Structure containing parameters specific to cadence watchdog.
+ */
+struct cdns_wdt {
+ void __iomem*regs;
+ bool
[PATCH v3 2/2] devicetree: Add Cadence WDT devicetree bindings documentation
Add cadence-wdt bindings documentation.
Signed-off-by: Harini Katakam
---
v3 changes:
- Change reset property type and improve description.
- Improve description of clocks and interrupts.
- Use watchdog@ in the example.
- Use only cdns compatible string for now.
v2:
No changes
---
.../devicetree/bindings/watchdog/cadence-wdt.txt | 27
1 file changed, 27 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
new file mode 100644
index 000..ab23e38
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
@@ -0,0 +1,27 @@
+Zynq Watchdog Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "cdns,wdt-r1p2".
+- clocks : Input clock specifier. This should be the ref clk.
+- reg : Physical base address and size of WDT registers map.
+- interrupts : Property with a value describing the interrupt
+ number. This interrupt is used for indication
+ when the watchdog times out.
+- interrupt-parent : Must be core interrupt controller.
+
+Optional properties
+- reset: If this property exists, then a reset is done
+ when watchdog times out.
+- timeout-sec : Watchdog timeout value (in seconds).
+
+Example:
+ watchdog@f8005000 {
+ compatible = "xlnx,zynq-wdt-r1p2";
+ clocks = <&clkc 45>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 9 1>;
+ reg = <0xf8005000 0x1000>;
+ reset;
+ timeout-sec = <10>;
+ };
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] devicetree: Add Cadence WDT devicetree bindings documentation
Hi Mark, On Mon, Jul 14, 2014 at 8:37 PM, Mark Rutland wrote: > On Mon, Jul 14, 2014 at 01:16:09PM +0100, Harini Katakam wrote: >> Add cadence-wdt bindings documentation. >> >> Signed-off-by: Harini Katakam >> --- >> >> v3 changes: >> - Change reset property type and improve description. >> - Improve description of clocks and interrupts. >> - Use watchdog@ in the example. >> - Use only cdns compatible string for now. >> >> v2: >> No changes >> >> --- >> .../devicetree/bindings/watchdog/cadence-wdt.txt | 27 >> >> 1 file changed, 27 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/watchdog/cadence-wdt.txt >> >> diff --git a/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt >> b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt >> new file mode 100644 >> index 000..ab23e38 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt >> @@ -0,0 +1,27 @@ >> +Zynq Watchdog Device Tree Bindings >> +--- >> + >> +Required properties: >> +- compatible : Should be "cdns,wdt-r1p2". >> +- clocks : Input clock specifier. This should be the ref clk. > > This wording makes it sound like the watchdog block has more than one > clock input. Does it? It doesn't. It has one APB clock - named pclk. > >> +- reg: Physical base address and size of WDT >> registers map. >> +- interrupts : Property with a value describing the interrupt >> + number. This interrupt is used for indication >> + when the watchdog times out. > > Just say "the watchdog timeout interrupt", or (better) use the name of > the interrupt from the documentation. OK. Yeah I'll do that. The interrupt name is wd_irq. > >> +- interrupt-parent : Must be core interrupt controller. >> + >> +Optional properties >> +- reset : If this property exists, then a reset is done >> + when watchdog times out. > > That's a bit of an ambiguous name (too easy to misconstrue as a reset > device reference). Do any other watchdogs have similar properties? I could change it to "reset-on-timeout" if that better. >From the documentation of other drivers, there seems to be a reset-type property in atmel. Dint find any other reset related properties. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] devicetree: Add Cadence WDT devicetree bindings documentation
Hi Mark, On Tue, Jul 15, 2014 at 2:29 PM, Mark Rutland wrote: > On Tue, Jul 15, 2014 at 07:39:40AM +0100, Harini Katakam wrote: >> Hi Mark, >> >> On Mon, Jul 14, 2014 at 8:37 PM, Mark Rutland wrote: >> > On Mon, Jul 14, 2014 at 01:16:09PM +0100, Harini Katakam wrote: >> >> Add cadence-wdt bindings documentation. >> >> >> >> Signed-off-by: Harini Katakam >> >> --- >> >> >> >> v3 changes: >> >> - Change reset property type and improve description. >> >> - Improve description of clocks and interrupts. >> >> - Use watchdog@ in the example. >> >> - Use only cdns compatible string for now. >> >> >> >> v2: >> >> No changes >> >> >> >> --- >> >> .../devicetree/bindings/watchdog/cadence-wdt.txt | 27 >> >> >> >> 1 file changed, 27 insertions(+) >> >> create mode 100644 >> >> Documentation/devicetree/bindings/watchdog/cadence-wdt.txt >> >> >> >> +Optional properties >> >> +- reset : If this property exists, then a reset is >> >> done >> >> + when watchdog times out. >> > >> > That's a bit of an ambiguous name (too easy to misconstrue as a reset >> > device reference). Do any other watchdogs have similar properties? >> >> I could change it to "reset-on-timeout" if that better. >> From the documentation of other drivers, there seems to be a reset-type >> property in atmel. Dint find any other reset related properties. >> > > Ok. reset-on-timeout sounds like a better property name to avoid > possible confusion. > > That said, what happens if we don't specify the device should reset the > system (but have a timeout-sec property)? > A reset wont be done but a message will be printed when the watchdog times out just as an indication. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 2/2] devicetree: Add devicetree bindings documentation for Zynq QSPI
Add bindings documentation for Zynq QSPI driver.
Signed-off-by: Harini Katakam
---
.../devicetree/bindings/spi/spi-zynq-qspi.txt | 28
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
b/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
new file mode 100644
index 000..288c551
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt
@@ -0,0 +1,28 @@
+Xilinx Zynq QSPI controller Device Tree Bindings
+-
+
+Required properties:
+- compatible : Should be "xlnx,zynq-qspi-1.0".
+- reg : Physical base address and size of QSPI registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller
+- clock-names : List of input clock names - "ref_clk", "pclk"
+ (See clock bindings for details).
+- clocks : Clock phandles (see clock bindings for details).
+
+Optional properties:
+- num-cs : Number of chip selects used.
+- xlnx,qspi-mode : Single - 0; Dual Stacked - 1; Dual Parallel - 2.
+
+Example:
+ qspi@e000d000 {
+ compatible = "xlnx,zynq-qspi-1.0";
+ clock-names = "ref_clk", "pclk";
+ clocks = <&clkc 10>, <&clkc 43>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 19 4>;
+ num-cs = <1>;
+ xlnx,qspi-mode = <0x0>;
+ reg = <0xe000d000 0x1000>;
+ } ;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
This patch adds support for QSPI controller used by Zynq. Signed-off-by: Harini Katakam --- drivers/spi/Kconfig |6 + drivers/spi/Makefile|1 + drivers/spi/spi-zynq-qspi.c | 854 +++ 3 files changed, 861 insertions(+) create mode 100644 drivers/spi/spi-zynq-qspi.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 213b5cb..9642148 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -558,6 +558,12 @@ config SPI_XTENSA_XTFPGA start and deasserting on end. +config SPI_ZYNQ_QSPI + tristate "Xilinx Zynq QSPI controller" + depends on ARCH_ZYNQ + help + This selects the Xilinx Zynq Quad SPI controller master driver. + config SPI_NUC900 tristate "Nuvoton NUC900 series SPI" depends on ARCH_W90X900 diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 929c9f5..0bfe75e 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -83,3 +83,4 @@ obj-$(CONFIG_SPI_TXX9)+= spi-txx9.o obj-$(CONFIG_SPI_XCOMM)+= spi-xcomm.o obj-$(CONFIG_SPI_XILINX) += spi-xilinx.o obj-$(CONFIG_SPI_XTENSA_XTFPGA)+= spi-xtensa-xtfpga.o +obj-$(CONFIG_SPI_ZYNQ_QSPI)+= spi-zynq-qspi.o diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c new file mode 100644 index 000..2a352a9 --- /dev/null +++ b/drivers/spi/spi-zynq-qspi.c @@ -0,0 +1,854 @@ +/* + * Xilinx Zynq Quad-SPI (QSPI) controller driver (master mode only) + * + * Copyright (C) 2009 - 2014 Xilinx, Inc. + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License version 2 as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Name of this driver */ +#define DRIVER_NAME"zynq-qspi" + +/* Register offset definitions */ +#define ZYNQ_QSPI_CONFIG_OFFSET0x00 /* Configuration Register, RW */ +#define ZYNQ_QSPI_STATUS_OFFSET0x04 /* Interrupt Status Register, RO */ +#define ZYNQ_QSPI_IEN_OFFSET 0x08 /* Interrupt Enable Register, WO */ +#define ZYNQ_QSPI_IDIS_OFFSET 0x0C /* Interrupt Disable Reg, WO */ +#define ZYNQ_QSPI_IMASK_OFFSET 0x10 /* Interrupt Enabled Mask Reg,RO */ +#define ZYNQ_QSPI_ENABLE_OFFSET0x14 /* Enable/Disable Register, RW */ +#define ZYNQ_QSPI_DELAY_OFFSET 0x18 /* Delay Register, RW */ +#define ZYNQ_QSPI_TXD_00_00_OFFSET 0x1C /* Transmit 4-byte inst, WO */ +#define ZYNQ_QSPI_TXD_00_01_OFFSET 0x80 /* Transmit 1-byte inst, WO */ +#define ZYNQ_QSPI_TXD_00_10_OFFSET 0x84 /* Transmit 2-byte inst, WO */ +#define ZYNQ_QSPI_TXD_00_11_OFFSET 0x88 /* Transmit 3-byte inst, WO */ +#define ZYNQ_QSPI_RXD_OFFSET 0x20 /* Data Receive Register, RO */ +#define ZYNQ_QSPI_SIC_OFFSET 0x24 /* Slave Idle Count Register, RW */ +#define ZYNQ_QSPI_TX_THRESH_OFFSET 0x28 /* TX FIFO Watermark Reg, RW */ +#define ZYNQ_QSPI_RX_THRESH_OFFSET 0x2C /* RX FIFO Watermark Reg, RW */ +#define ZYNQ_QSPI_GPIO_OFFSET 0x30 /* GPIO Register, RW */ +#define ZYNQ_QSPI_LINEAR_CFG_OFFSET0xA0 /* Linear Adapter Config Ref, RW */ +#define ZYNQ_QSPI_MOD_ID_OFFSET0xFC /* Module ID Register, RO */ + +/* + * QSPI Configuration Register bit Masks + * + * This register contains various control bits that effect the operation + * of the QSPI controller + */ +#define ZYNQ_QSPI_CONFIG_IFMODE_MASK 0x8000 /* Flash Memory Interface */ +#define ZYNQ_QSPI_CONFIG_MANSRT_MASK 0x0001 /* Manual TX Start */ +#define ZYNQ_QSPI_CONFIG_MANSRTEN_MASK 0x8000 /* Enable Manual TX Mode */ +#define ZYNQ_QSPI_CONFIG_SSFORCE_MASK 0x4000 /* Manual Chip Select */ +#define ZYNQ_QSPI_CONFIG_BDRATE_MASK 0x0038 /* Baud Rate Divisor Mask */ +#define ZYNQ_QSPI_CONFIG_CPHA_MASK 0x0004 /* Clock Phase Control */ +#define ZYNQ_QSPI_CONFIG_CPOL_MASK 0x0002 /* Clock Polarity Control */ +#define ZYNQ_QSPI_CONFIG_SSCTRL_MASK 0x3C00 /* Slave Select Mask */ +#define ZYNQ_QSPI_CONFIG_FWIDTH_MASK 0x00C0 /* FIFO width */ +#define ZYNQ_QSPI_CONFIG_MSTREN_MASK 0x0001 /* Master Mode */ + +/* + * QSPI Configuration Register - Baud rate and slave select + * + * These are the values used in the calculation of baud rate divisor and + * setting the slave select. + */ +#define ZYNQ_QSPI_BAUD_DIV_MAX 7 /* Baud rate divisor maximum */ +#define ZYNQ_QSPI_BAUD_DIV_SHIFT 3 /* Baud rate divisor shift in CR */ +#define ZYNQ_QSPI_SS_SHIFT 10 /* Slave Select field shift in CR */ + +/* + * QSPI Interrupt Registers bit Masks + * + * All the four interrupt registers (Status/Mask
[RFC PATCH 0/2] Zynq QSPI RFC
Xilinx Zynq uses a QSPI controller that is based on the Cadence SPI IP. This controller implements all the functionality required to support Quad SPI NOR flash devices. This driver along with the MTD layer is used to support flash devices. This series is for the following purposes: - RFC of the Quad SPI driver. - We currently use a custom MTD layer and would like to get inputs on dual stacked/dual parallel handling (described below). The flash device(s) can be connected in the three configurations: 1. Single - One flash device with 1 CS, 1 Clock and 4 IO lines. 2. Dual Parallel - Two flash devices connected with common CS and separate IO lines (resulting in 8 IO lines). In this configuration, the controller a) Duplicates commands, address etc. sent on both sets of 4 IO lines. b) Stripes data both transmitted and received i.e. 4 bits of data is sent to the first flash and the other 4 bits to the second flash. Similarly read data is also consolidated. Due to this, TX and RX data handling in the driver need special handling for parallel mode. 3. Dual Stacked - Two flash devices connected with separate CS and 4 common IO lines. This is largely similar to single, except for the slave selection logic. The above configuration is conveyed to the QSPI driver through a devicetree property. The QSPI driver differs from the existing Cadence SPI driver in the following respects majorly: 1. TX and RX handling: Different TX registers are used to write into the TX FIFO. TXD0, TXD1, TXD2 and TXD3 are used write 4, 1, 2 and 3 bytes respectively. Depending on the TXD register used, the received bytes also need to be handled separately. 2. Depending on the configuration in which flash devices are connected (single, parallel or stacked), QSPI controller configuration registers need to be modified. 3. There is no support for extended slave select in QSPI, as opposed to SPI. In case of stacked configuration, the slave select field remains the same and a different configuration bit is used to select between the two flash devices. 4. Handling of dual parallel configuration. MTD layer: The Xilinx Zynq MTD layer by far makes use of the mainline version with some differences. The primary flash families supported are Spansion, Winbond and Micron. - Probe: - In dual configurations, both flash devices are recognized as one continuous memory. (ID is read only from one flash and it is a pre-stated assumption that both flash devices have the same flash make and size.) - Addressing: a) In dual stacked mode, the address passed to the MTD layer can be between 0x0 to 2*(one flash size). Hence the MTD layer has to recognize whether the address belongs to the first flash or the second flash subtract the offset and indicate the same to the QSPI driver. b) In dual stacked mode too the address can range between 0 to 2*(one flash size). But, when an 8 bit word is written, 4 bits are written to the first and 4 bits are written to the second flash. Hence the address sent is always halved and checks are in place for even address and even length. - 4 byte addressing is not supported and hence bank selection logic is used along with the addressing system described above. - Flash register read/writes, for example, lock/unlock, quad enable etc. are handled differently in dual stacked and parallel modes. I'm sorry for the long cover letter. Hope it helps. Harini Katakam (2): spi: Add support for Zynq QSPI controller devicetree: Add devicetree bindings documentation for Zynq QSPI .../devicetree/bindings/spi/spi-zynq-qspi.txt | 28 + drivers/spi/Kconfig|6 + drivers/spi/Makefile |1 + drivers/spi/spi-zynq-qspi.c| 854 4 files changed, 889 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-zynq-qspi.txt create mode 100644 drivers/spi/spi-zynq-qspi.c -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] gpio: Add driver for Zynq GPIO controller
On Thu, Jul 10, 2014 at 2:42 PM, Linus Walleij wrote: > On Tue, Jul 8, 2014 at 1:02 PM, Harini Katakam wrote: > >> From: Harini Katakam >> >> Add support for GPIO controller used by Xilinx Zynq. >> >> Signed-off-by: Harini Katakam >> Signed-off-by: Soren Brinkmann > > Patch applied! Thanks for your patient work on this Harini! > Thanks Linus. @Soren Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
Hi Geert, On Thu, Jul 10, 2014 at 2:48 PM, Geert Uytterhoeven wrote: > Hi Harini, > > On Thu, Jul 10, 2014 at 10:50 AM, Harini Katakam wrote: >> + master->flags = SPI_MASTER_QUAD_MODE; > > SPI_MASTER_QUAD_MODE is not one of the SPI_MASTER_* defines > in include/linux/spi/spi.h? > I'm sorry about that. That flag is unused - will remove this statement. >> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD | >> + SPI_TX_DUAL | SPI_TX_QUAD; > > Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't > check spi_transfer.[tr]x_nbits? How can it determine when to enable Dual/Quad? > Here the driver is just giving information that the controller support it. The MTD layer enables dual/quad based on what the flash supports; quad being the first priority I understand that the spi core reads rx, tx-bus-width property and master support flags and performs the necessary checks. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
Hi Geert, On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven wrote: > Hi Harini, > > On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam > wrote: >>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | >>>> SPI_RX_QUAD | >>>> + SPI_TX_DUAL | SPI_TX_QUAD; >>> >>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't >>> check spi_transfer.[tr]x_nbits? How can it determine when to enable >>> Dual/Quad? >> >> Here the driver is just giving information that the controller support it. >> The MTD layer enables dual/quad based on what the flash supports; quad >> being the first priority >> I understand that the spi core reads rx, tx-bus-width property and >> master support flags and >> performs the necessary checks. > > That's correct: as long as the rx, tx-bus-width properties do not indicate a > Dual or Quad wiring, it won't be used. > > However, based on schematics, someone may set the rx, tx-bus-width properties > to 4, which is correct, as DT describes the hardware. But this will fail to > work. > So I think it's safer not to announce Dual/Quad support in the driver until > the actual driver support is there. > OK. Correct me if I'm wrong but announcing this support in master->flags is just to say the controller supports it - Like Punnaiah mentioned in the other mail, nothing specific needs to be done from the controller driver to enable dual/quad support. This is at the SOC/IP level. I agree it might or might not be supported at board-level. But that's based on the user's hardware. Should master->flags really take this into consideration? BTW, I dint see master->mode_bits being used anywhere at the moment. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
Hi Geert, On Thu, Jul 10, 2014 at 4:55 PM, Geert Uytterhoeven wrote: > Hi Harini, > > On Thu, Jul 10, 2014 at 12:33 PM, Harini Katakam > wrote: >> On Thu, Jul 10, 2014 at 3:12 PM, Geert Uytterhoeven >> wrote: >>> On Thu, Jul 10, 2014 at 11:31 AM, Harini Katakam >>> wrote: >>>>>> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | >>>>>> SPI_RX_QUAD | >>>>>> + SPI_TX_DUAL | SPI_TX_QUAD; >>>>> >>>>> Your driver advertises Dual/Quad SPI Transfer capabilities, but it doesn't >>>>> check spi_transfer.[tr]x_nbits? How can it determine when to enable >>>>> Dual/Quad? >>>> >>>> Here the driver is just giving information that the controller support it. >>>> The MTD layer enables dual/quad based on what the flash supports; quad >>>> being the first priority >>>> I understand that the spi core reads rx, tx-bus-width property and >>>> master support flags and >>>> performs the necessary checks. >>> >>> That's correct: as long as the rx, tx-bus-width properties do not indicate >>> a >>> Dual or Quad wiring, it won't be used. >>> >>> However, based on schematics, someone may set the rx, tx-bus-width >>> properties >>> to 4, which is correct, as DT describes the hardware. But this will fail to >>> work. >>> So I think it's safer not to announce Dual/Quad support in the driver until >>> the actual driver support is there. >> >> OK. Correct me if I'm wrong but announcing this support in master->flags is >> just to say the controller supports it - Like Punnaiah mentioned in the other >> mail, nothing specific needs to be done from the controller driver to enable >> dual/quad support. This is at the SOC/IP level. >> I agree it might or might not be supported at board-level. > > IC. So this is not a generic SPI controller, but a controller meant for QSPI > FLASHes? I.e. if you would connect a different device, the controller may > unexpectedly use Dual or Quad mode if it sees a byte fly by that looks like > a Quad SPI FLASH read command? > Yes. It would. >> But that's based on the user's hardware. Should master->flags >> really take this into consideration? > > You mean master->mode_bits? Yes, i mean mode_bits. That was typo. > >> BTW, I dint see master->mode_bits being used anywhere at the moment. > > It is used to match SPI controller and slave features, cfr. spi_setup() in > drivers/spi/spi.c. > > If Dual/Quad is supported, the bits should be set. Else spi_setup() will > clear the bits in the SPI slave's mode field, disabling Dual/Quad transfers. > OK. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/2] spi: Add support for Zynq QSPI controller
Hi Mark, On Thu, Jul 10, 2014 at 5:31 PM, Mark Brown wrote: > On Thu, Jul 10, 2014 at 04:03:19PM +0530, Harini Katakam wrote: > >> OK. Correct me if I'm wrong but announcing this support in master->flags is >> just to say the controller supports it - Like Punnaiah mentioned in the other > > No, it's broken to set this if there is no ability to use it. > >> mail, nothing specific needs to be done from the controller driver to enable >> dual/quad support. This is at the SOC/IP level. > > How does the client driver select the width to use for a transfer? This controller is meant to be used only with flash devices. The flash devices' supported width will be reflected in a table in MTD layer. When selecting, priority is given to quad over dual and single in the MTD and it will send commands using the supported tx/rx bus width accordingly. About the supported bus width on board, tx-bus-width and rx-bus-width properties in dts will have the info; which I believe spi core uses. Regards, Harini -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] gpio: Add driver for Zynq GPIO controller
From: Harini Katakam Add support for GPIO controller used by Xilinx Zynq. Signed-off-by: Harini Katakam Signed-off-by: Soren Brinkmann --- v3 changes: - Use linux/gpio/driver.h instead of linux/gpio.h - Make irq a local variable in probe v2 changes: - convert to pm_runtime_force_(suspend|resume) - add pm_runtime_set_active in probe() - also (un)prepare clocks when they are dis-/enabled - add some missing calls to pm_runtime_get() - use pm_runtime_put() instead of sync variant - remove gpio chip in driver remove() - remove redundant type casts - directly use IO helpers - use BIT macro to set/clear bits - migrate to GPIOLIB_IRQCHIP --- drivers/gpio/Kconfig |7 + drivers/gpio/Makefile|1 + drivers/gpio/gpio-zynq.c | 649 ++ 3 files changed, 657 insertions(+) create mode 100644 drivers/gpio/gpio-zynq.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 4a1b511..bdeef02 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -340,6 +340,13 @@ config GPIO_XILINX help Say yes here to support the Xilinx FPGA GPIO device +config GPIO_ZYNQ + tristate "Xilinx Zynq GPIO support" + depends on ARCH_ZYNQ + select GPIOLIB_IRQCHIP + help + Say yes here to support Xilinx Zynq GPIO controller. + config GPIO_XTENSA bool "Xtensa GPIO32 support" depends on XTENSA diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index d10f6a9..033fd7c 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -101,3 +101,4 @@ obj-$(CONFIG_GPIO_WM8994) += gpio-wm8994.o obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o obj-$(CONFIG_GPIO_ZEVIO) += gpio-zevio.o +obj-$(CONFIG_GPIO_ZYNQ)+= gpio-zynq.o diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c new file mode 100644 index 000..c0c53fd --- /dev/null +++ b/drivers/gpio/gpio-zynq.c @@ -0,0 +1,649 @@ +/* + * Xilinx Zynq GPIO device driver + * + * Copyright (C) 2009 - 2014 Xilinx, Inc. + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 2 of the License, or (at your option) any later + * version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_NAME "zynq-gpio" + +/* Maximum banks */ +#define ZYNQ_GPIO_MAX_BANK 4 + +#define ZYNQ_GPIO_BANK0_NGPIO 32 +#define ZYNQ_GPIO_BANK1_NGPIO 22 +#define ZYNQ_GPIO_BANK2_NGPIO 32 +#define ZYNQ_GPIO_BANK3_NGPIO 32 + +#define ZYNQ_GPIO_NR_GPIOS (ZYNQ_GPIO_BANK0_NGPIO + \ +ZYNQ_GPIO_BANK1_NGPIO + \ +ZYNQ_GPIO_BANK2_NGPIO + \ +ZYNQ_GPIO_BANK3_NGPIO) + +#define ZYNQ_GPIO_BANK0_PIN_MIN0 +#define ZYNQ_GPIO_BANK0_PIN_MAX(ZYNQ_GPIO_BANK0_PIN_MIN + \ + ZYNQ_GPIO_BANK0_NGPIO - 1) +#define ZYNQ_GPIO_BANK1_PIN_MIN(ZYNQ_GPIO_BANK0_PIN_MAX + 1) +#define ZYNQ_GPIO_BANK1_PIN_MAX(ZYNQ_GPIO_BANK1_PIN_MIN + \ + ZYNQ_GPIO_BANK1_NGPIO - 1) +#define ZYNQ_GPIO_BANK2_PIN_MIN(ZYNQ_GPIO_BANK1_PIN_MAX + 1) +#define ZYNQ_GPIO_BANK2_PIN_MAX(ZYNQ_GPIO_BANK2_PIN_MIN + \ + ZYNQ_GPIO_BANK2_NGPIO - 1) +#define ZYNQ_GPIO_BANK3_PIN_MIN(ZYNQ_GPIO_BANK2_PIN_MAX + 1) +#define ZYNQ_GPIO_BANK3_PIN_MAX(ZYNQ_GPIO_BANK3_PIN_MIN + \ + ZYNQ_GPIO_BANK3_NGPIO - 1) + + +/* Register offsets for the GPIO device */ +/* LSW Mask & Data -WO */ +#define ZYNQ_GPIO_DATA_LSW_OFFSET(BANK)(0x000 + (8 * BANK)) +/* MSW Mask & Data -WO */ +#define ZYNQ_GPIO_DATA_MSW_OFFSET(BANK)(0x004 + (8 * BANK)) +/* Data Register-RW */ +#define ZYNQ_GPIO_DATA_RO_OFFSET(BANK) (0x060 + (4 * BANK)) +/* Direction mode reg-RW */ +#define ZYNQ_GPIO_DIRM_OFFSET(BANK)(0x204 + (0x40 * BANK)) +/* Output enable reg-RW */ +#define ZYNQ_GPIO_OUTEN_OFFSET(BANK) (0x208 + (0x40 * BANK)) +/* Interrupt mask reg-RO */ +#define ZYNQ_GPIO_INTMASK_OFFSET(BANK) (0x20C + (0x40 * BANK)) +/* Interrupt enable reg-WO */ +#define ZYNQ_GPIO_INTEN_OFFSET(BANK) (0x210 + (0x40 * BANK)) +/* Interrupt disable reg-WO */ +#define ZYNQ_GPIO_INTDIS_OFFSET(BANK) (0x214 + (0x40 * BANK)) +/* Interrupt status reg-RO */ +#define ZYNQ_GPIO_INTSTS_OFFSET(BANK) (0x218 + (0x40 * BANK)) +/* Interrupt type reg-RW */ +#define ZYNQ_GPIO_INTTYPE_OFFSET(BANK) (0x21C + (0x40 * BANK)) +/* Interrupt polarity reg-RW */ +#define ZYNQ_GPIO_INTPOL_OFFSET(BANK) (0x220 + (0x40 * BANK)) +/* Interrupt on any, reg-RW */ +#define ZYNQ_GPIO_INTANY_OFFSET(BANK) (0x224 + (0x40 * BANK)) + +/* Disable all interrupts mask
[PATCH v3 2/2] devicetree: Add Zynq GPIO devicetree bindings documentation
From: Harini Katakam
Add gpio-zynq bindings documentation.
Signed-off-by: Harini Katakam
Signed-off-by: Soren Brinkmann
---
v3 changes:
Change description of #gpio-cells - use 'gpio-line' instead of 'pin'
v2 changes:
Improve description.
---
.../devicetree/bindings/gpio/gpio-zynq.txt | 26
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-zynq.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
new file mode 100644
index 000..986371a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
@@ -0,0 +1,26 @@
+Xilinx Zynq GPIO controller Device Tree Bindings
+---
+
+Required properties:
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+- compatible : Should be "xlnx,zynq-gpio-1.0"
+- clocks : Clock specifier (see clock bindings for details)
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+- interrupt-parent : Must be core interrupt controller
+- reg : Address and length of the register set for the device
+
+Example:
+ gpio@e000a000 {
+ #gpio-cells = <2>;
+ compatible = "xlnx,zynq-gpio-1.0";
+ clocks = <&clkc 42>;
+ gpio-controller;
+ interrupt-parent = <&intc>;
+ interrupts = <0 20 4>;
+ reg = <0xe000a000 0x1000>;
+ };
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] devicetree: Add Cadence WDT devicetree bindings documentation
Add cadence-wdt bindings documentation.
Signed-off-by: Harini Katakam
---
.../devicetree/bindings/watchdog/cadence-wdt.txt | 26
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
new file mode 100644
index 000..1f7a732
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/cadence-wdt.txt
@@ -0,0 +1,26 @@
+Zynq Watchdog Device Tree Bindings
+---
+
+Required properties:
+- compatible : Should be "xlnx,zynq-wdt-r1p2" or "cdns,wdt-r1p2".
+- clocks : Clock phandles (see clock bindings for details).
+- reg : Physical base address and size of WDT registers map.
+- interrupts : Property with a value describing the interrupt
+ number.
+- interrupt-parent : Must be core interrupt controller.
+
+Optional properties
+- reset: Reset interrupt.
+- timeout-sec : Watchdog timeout value (in seconds).
+
+Example:
+
+ wdt@f8005000 {
+ compatible = "xlnx,zynq-wdt-r1p2";
+ clocks = <&clkc 45>;
+ interrupt-parent = <&intc>;
+ interrupts = <0 9 1>;
+ reg = <0xf8005000 0x1000>;
+ reset = <0>;
+ timeout-sec = <10>;
+ } ;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] watchdog: Add Cadence WDT driver
Add Cadence WDT driver. This is used by Xilinx Zynq.
Signed-off-by: Harini Katakam
---
drivers/watchdog/Kconfig |7 +
drivers/watchdog/Makefile |1 +
drivers/watchdog/cadence_wdt.c | 530
3 files changed, 538 insertions(+)
create mode 100644 drivers/watchdog/cadence_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 79d2589..13c61d8 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -138,6 +138,13 @@ config AT91SAM9X_WATCHDOG
Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will
reboot your system when the timeout is reached.
+config CADENCE_WATCHDOG
+ tristate "Cadence Watchdog Timer"
+ select WATCHDOG_CORE
+ help
+ Say Y here if you want to include support for the watchdog
+ timer in the Xilinx Zynq.
+
config 21285_WATCHDOG
tristate "DC21285 watchdog"
depends on FOOTBRIDGE
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 985a66c..49e3e77 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
+obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o
obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
obj-$(CONFIG_TWL4030_WATCHDOG) += twl4030_wdt.o
obj-$(CONFIG_21285_WATCHDOG) += wdt285.o
diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
new file mode 100644
index 000..01e8c78
--- /dev/null
+++ b/drivers/watchdog/cadence_wdt.c
@@ -0,0 +1,530 @@
+/*
+ * Cadence WDT driver - Used by Xilinx Zynq
+ *
+ * Copyright (C) 2010 - 2014 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define CDNS_WDT_DEFAULT_TIMEOUT 10
+/* Supports 1 - 516 sec */
+#define CDNS_WDT_MIN_TIMEOUT 1
+#define CDNS_WDT_MAX_TIMEOUT 516
+
+/* Restart key */
+#define CDNS_WDT_RESTART_KEY 0x1999
+
+/* Counter register access key */
+#define CDNS_WDT_REGISTER_ACCESS_KEY 0x0092
+
+/* Counter value divisor */
+#define CDNS_WDT_COUNTER_VALUE_DIVISOR 0x1000
+
+/* Clock prescaler value and selection */
+#define CDNS_WDT_PRESCALE_64 64
+#define CDNS_WDT_PRESCALE_512 512
+#define CDNS_WDT_PRESCALE_4096 4096
+#define CDNS_WDT_PRESCALE_SELECT_641
+#define CDNS_WDT_PRESCALE_SELECT_512 2
+#define CDNS_WDT_PRESCALE_SELECT_4096 3
+
+/* Input clock frequency */
+#define CDNS_WDT_CLK_10MHZ 1000
+#define CDNS_WDT_CLK_75MHZ 7500
+
+/* Counter maximum value */
+#define CDNS_WDT_COUNTER_MAX 0xFFF
+
+static int wdt_timeout = CDNS_WDT_DEFAULT_TIMEOUT;
+static int nowayout = WATCHDOG_NOWAYOUT;
+
+module_param(wdt_timeout, int, 0);
+MODULE_PARM_DESC(wdt_timeout,
+"Watchdog time in seconds. (default="
+__MODULE_STRING(CDNS_WDT_DEFAULT_TIMEOUT) ")");
+
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout,
+"Watchdog cannot be stopped once started (default="
+__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+/**
+ * struct cdns_wdt - Watchdog device structure
+ * @regs: baseaddress of device
+ * @rst: reset flag
+ * @clk: struct clk * of a clock source
+ * @prescaler: for saving prescaler value
+ * @ctrl_clksel: counter clock prescaler selection
+ * @io_lock: spinlock for IO register access
+ * @cdns_wdt_device: watchdog device structure
+ * @cdns_wdt_notifier: notifier structure
+ *
+ * Structure containing parameters specific to cadence watchdog.
+ */
+struct cdns_wdt {
+ void __iomem*regs;
+ u32 rst;
+ struct clk *clk;
+ u32 prescaler;
+ u32 ctrl_clksel;
+ spinlock_t io_lock;
+ struct watchdog_device cdns_wdt_device;
+ struct notifier_block cdns_wdt_notifier;
+};
+
+/* Write access to Registers */
+static inline void cdns_wdt_writereg(void __iomem *offset, u32 val)
+{
+ writel_relaxed(val, offset);
+}
+
+/*Register Map**/
+
+/* Register Offsets for the WDT */
+#define CDNS_WDT_ZMR_OFFSET0x0 /* Zero Mode Register */
+#define CDNS_WDT_CCR_OFFSET0x4 /* Counter Control Register */
+#define CDNS_WDT_RESTART_OFFSET0x8 /* Restart Register */
+#define CDNS_WDT_SR_OFFSET 0xC /* Status Register */
+
+/*
+ * Zero Mode Register - This register controls ho
