RE: [PATCH] drivers: ata: Move vendor specific sata port phy oob settings to device-tree

2015-12-09 Thread Anurag Kumar Vulisha
Hi Rob,
Do you have any further comments on this  or shall we proceed with this 
patch .
Thanks,
Anurag Kumar V

> -Original Message-
> From: Anurag Kumar Vulisha
> Sent: Tuesday, November 17, 2015 7:54 PM
> To: 'Rob Herring'
> Cc: pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; t...@kernel.org;
> devicetree@vger.kernel.org; linux-ker...@vger.kernel.org; linux-
> i...@vger.kernel.org; Michal Simek; Punnaiah Choudary Kalluri; Anirudha
> Sarangi; Srikanth Vemula
> Subject: RE: [PATCH] drivers: ata: Move vendor specific sata port phy oob
> settings to device-tree
>
> Hi Rob,
>
>
> Thanks for reviewing the patch
>
> > -Original Message-
> > From: Rob Herring [mailto:r...@kernel.org]
> > Sent: Friday, November 13, 2015 8:02 PM
> > To: Anurag Kumar Vulisha
> > Cc: pawel.m...@arm.com; mark.rutl...@arm.com;
> > ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; t...@kernel.org;
> > devicetree@vger.kernel.org; linux-ker...@vger.kernel.org; linux-
> > i...@vger.kernel.org; Michal Simek; Punnaiah Choudary Kalluri; Anirudha
> > Sarangi; Srikanth Vemula; Anurag Kumar Vulisha
> > Subject: Re: [PATCH] drivers: ata: Move vendor specific sata port phy
> > oob settings to device-tree
> >
> > On Fri, Nov 13, 2015 at 04:02:23PM +0530, Anurag Kumar Vulisha wrote:
> > > In SATA, speed negotiation happens with  OOB(Out of Band) signals.
> > > These OOB signal timing values are configured through vendor
> > > specific registers in the SATA controller. These OOB timings depends
> > > on the generator and detector clock frequency, which varies from
> > > board to board (ex: ep108, zcu102 and zc1751 has different clock
> frequencies).
> >
> > Could you calculate the timings based on the frequency instead?
> >
>
> We can calculate the OOB timing settings based on frequency, but it requires
> complex calculations and floating point operations  in the sata driver , 
> which is
> running in kernel mode. To my knowledge it is not recommended to do
> floating  operations inside kernel mode(Please correct me if I wrong ) .
> Because of this reason we are reading those values from  device tree.
>
> > > Since to make ahci_ceva driver generic, it would be better to move
> > > these settings to the device-tree node and read them from driver.
> > >
> > > This patch does the same.
> > >
> > > Signed-off-by: Anurag Kumar Vulisha 
> > > ---
> > >  .../devicetree/bindings/ata/ahci-ceva.txt  |   38 +
> > >  drivers/ata/ahci_ceva.c|   84 
> > > ++--
> > >  2 files changed, 99 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > > b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > > index 7ca8b97..66fcd10 100644
> > > --- a/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > > +++ b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > > @@ -5,6 +5,36 @@ Required properties:
> > >- compatible: Compatibility string. Must be 'ceva,ahci-1v84'.
> > >- clocks: Input clock specifier. Refer to common clock bindings.
> > >- interrupts: Interrupt specifier. Refer to interrupt binding.
> > > +- ceva,p0-cominit-params: OOB timing value for COMINIT
> > > + parameter for
> > port 0.
> > > +- ceva,p1-cominit-params: OOB timing value for COMINIT
> > > + parameter for
> > port 1.
> >
> > This doesn't really scale when you have more than 2 ports. Given that
> > you know the length for each port, just make it a single property (i.e.
> > an array of ports).
> >
> The ceva sata controller that we are using  has support for 2 ports only and
> each port can Invidually configured to have different timing settings based on
> frequency. So for ease of use we  thought of having separate DT parameters
> for port 0 and 1 .
>
> > > + The fields for the above parameter must be as 
> > > shown
> > below:
> > > + ceva,phy-cominit-params = /bits/ 8  > CIBGMX CIBGN CINMP>;
> > > + CINMP : COMINIT Negate Minimum Period.
> > > + CIBGN : COMINIT Burst Gap Nominal.
> > > + CIBGMX: COMINIT Burst Gap Maximum.
> > > + CIBGMN: COMINIT Burst Gap Minimum.
> > > +- ceva,p0-comwake-params: OOB timing value for COMWAKE
> > p

RE: [PATCH] drivers: ata: Move vendor specific sata port phy oob settings to device-tree

2015-11-17 Thread Anurag Kumar Vulisha
Hi Rob,


Thanks for reviewing the patch

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Friday, November 13, 2015 8:02 PM
> To: Anurag Kumar Vulisha
> Cc: pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; t...@kernel.org;
> devicetree@vger.kernel.org; linux-ker...@vger.kernel.org; linux-
> i...@vger.kernel.org; Michal Simek; Punnaiah Choudary Kalluri; Anirudha
> Sarangi; Srikanth Vemula; Anurag Kumar Vulisha
> Subject: Re: [PATCH] drivers: ata: Move vendor specific sata port phy oob
> settings to device-tree
> 
> On Fri, Nov 13, 2015 at 04:02:23PM +0530, Anurag Kumar Vulisha wrote:
> > In SATA, speed negotiation happens with  OOB(Out of Band) signals.
> > These OOB signal timing values are configured through vendor specific
> > registers in the SATA controller. These OOB timings depends on the
> > generator and detector clock frequency, which varies from board to
> > board (ex: ep108, zcu102 and zc1751 has different clock frequencies).
> 
> Could you calculate the timings based on the frequency instead?
>

We can calculate the OOB timing settings based on frequency, but it requires 
complex calculations and floating 
point operations  in the sata driver , which is running in kernel mode. To my 
knowledge it is not recommended 
to do floating  operations inside kernel mode(Please correct me if I wrong ) . 
Because of this reason we are reading
those values from  device tree.
 
> > Since to make ahci_ceva driver generic, it would be better to move
> > these settings to the device-tree node and read them from driver.
> >
> > This patch does the same.
> >
> > Signed-off-by: Anurag Kumar Vulisha 
> > ---
> >  .../devicetree/bindings/ata/ahci-ceva.txt  |   38 +
> >  drivers/ata/ahci_ceva.c|   84 
> > ++--
> >  2 files changed, 99 insertions(+), 23 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > index 7ca8b97..66fcd10 100644
> > --- a/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > +++ b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
> > @@ -5,6 +5,36 @@ Required properties:
> >- compatible: Compatibility string. Must be 'ceva,ahci-1v84'.
> >- clocks: Input clock specifier. Refer to common clock bindings.
> >- interrupts: Interrupt specifier. Refer to interrupt binding.
> > +- ceva,p0-cominit-params: OOB timing value for COMINIT parameter for
> port 0.
> > +- ceva,p1-cominit-params: OOB timing value for COMINIT parameter for
> port 1.
> 
> This doesn't really scale when you have more than 2 ports. Given that you
> know the length for each port, just make it a single property (i.e.
> an array of ports).
> 
The ceva sata controller that we are using  has support for 2 ports only and 
each port can
Invidually configured to have different timing settings based on frequency. So 
for ease of
use we  thought of having separate DT parameters for port 0 and 1 .

> > +   The fields for the above parameter must be as shown
> below:
> > +   ceva,phy-cominit-params = /bits/ 8  CIBGMX CIBGN CINMP>;
> > +   CINMP : COMINIT Negate Minimum Period.
> > +   CIBGN : COMINIT Burst Gap Nominal.
> > +   CIBGMX: COMINIT Burst Gap Maximum.
> > +   CIBGMN: COMINIT Burst Gap Minimum.
> > +- ceva,p0-comwake-params: OOB timing value for COMWAKE
> parameter for port 0.
> > +- ceva,p1-comwake-params: OOB timing value for COMWAKE
> parameter for port 1.
> > +   The fields for the above parameter must be as shown
> below:
> > +   ceva,phy-comwake-params = /bits/ 8  CWBGMX CWBGN CWNMP>;
> > +   CWBGMN: COMWAKE Burst Gap Minimum.
> > +   CWBGMX: COMWAKE Burst Gap Maximum.
> > +   CWBGN: COMWAKE Burst Gap Nominal.
> > +   CWNMP: COMWAKE Negate Minimum Period.
> > +- ceva,p0-burst-params: Burst timing value for COM parameter for port
> 0.
> > +- ceva,p1-burst-params: Burst timing value for COM parameter for port
> 1.
> > +   The fields for the above parameter must be as shown
> below:
> > +   ceva,phy-burst-params = /bits/ 8  PTST>;
> > +   BMX: COM Burst Maximum.
> > +   BNM: COM Burst Nominal.
> > +   SFD: Signal Failure Detection value.
> > +   PTST: Partial 

[PATCH] drivers: ata: Move vendor specific sata port phy oob settings to device-tree

2015-11-13 Thread Anurag Kumar Vulisha
In SATA, speed negotiation happens with  OOB(Out of Band) signals. These OOB
signal timing values are configured through vendor specific registers in the
SATA controller. These OOB timings depends on the generator and detector clock
frequency, which varies from board to board (ex: ep108, zcu102 and zc1751 has
different clock frequencies).
Since to make ahci_ceva driver generic, it would be better to move these
settings to the device-tree node and read them from driver.

This patch does the same.

Signed-off-by: Anurag Kumar Vulisha 
---
 .../devicetree/bindings/ata/ahci-ceva.txt  |   38 +
 drivers/ata/ahci_ceva.c|   84 ++--
 2 files changed, 99 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/ata/ahci-ceva.txt 
b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
index 7ca8b97..66fcd10 100644
--- a/Documentation/devicetree/bindings/ata/ahci-ceva.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-ceva.txt
@@ -5,6 +5,36 @@ Required properties:
   - compatible: Compatibility string. Must be 'ceva,ahci-1v84'.
   - clocks: Input clock specifier. Refer to common clock bindings.
   - interrupts: Interrupt specifier. Refer to interrupt binding.
+- ceva,p0-cominit-params: OOB timing value for COMINIT parameter for port 
0.
+- ceva,p1-cominit-params: OOB timing value for COMINIT parameter for port 
1.
+   The fields for the above parameter must be as shown 
below:
+   ceva,phy-cominit-params = /bits/ 8 ;
+   CINMP : COMINIT Negate Minimum Period.
+   CIBGN : COMINIT Burst Gap Nominal.
+   CIBGMX: COMINIT Burst Gap Maximum.
+   CIBGMN: COMINIT Burst Gap Minimum.
+- ceva,p0-comwake-params: OOB timing value for COMWAKE parameter for port 
0.
+- ceva,p1-comwake-params: OOB timing value for COMWAKE parameter for port 
1.
+   The fields for the above parameter must be as shown 
below:
+   ceva,phy-comwake-params = /bits/ 8 ;
+   CWBGMN: COMWAKE Burst Gap Minimum.
+   CWBGMX: COMWAKE Burst Gap Maximum.
+   CWBGN: COMWAKE Burst Gap Nominal.
+   CWNMP: COMWAKE Negate Minimum Period.
+- ceva,p0-burst-params: Burst timing value for COM parameter for port 0.
+- ceva,p1-burst-params: Burst timing value for COM parameter for port 1.
+   The fields for the above parameter must be as shown 
below:
+   ceva,phy-burst-params = /bits/ 8 ;
+   BMX: COM Burst Maximum.
+   BNM: COM Burst Nominal.
+   SFD: Signal Failure Detection value.
+   PTST: Partial to Slumber timer value.
+- ceva,p0-retry-params: Retry interval timing value for port 0.
+- ceva,p1-retry-params: Retry interval timing value for port 1.
+   The fields for the above parameter must be as shown 
below:
+   ceva,phy-retry-params = /bits/ 16 ;
+   RIT:  Retry Interval Timer.
+   RCT:  Rate Change Timer.
 
 Optional properties:
   - ceva,broken-gen2: limit to gen1 speed instead of gen2.
@@ -17,4 +47,12 @@ Examples:
interrupts = <0 133 4>;
clocks = <&clkc SATA_CLK_ID>;
ceva,broken-gen2;
+   ceva,p0-cominit-params = /bits/ 8 <0x0F 0x25 0x18 0x29>;
+   ceva,p0-comwake-params = /bits/ 8 <0x04 0x0B 0x08 0x0F>;
+   ceva,p0-burst-params = /bits/ 8 <0x0A 0x08 0x4A 0x06>;
+   ceva,p0-retry-params = /bits/ 16 <0x0216 0x7F06>;
+   ceva,p1-cominit-params = /bits/ 8 <0x0F 0x25 0x18 0x29>;
+   ceva,p1-comwake-params = /bits/ 8 <0x04 0x0B 0x08 0x0F>;
+   ceva,p1-burst-params = /bits/ 8 <0x0A 0x08 0x4A 0x06>;
+   ceva,p1-retry-params = /bits/ 16 <0x0216 0x7F06>;
};
diff --git a/drivers/ata/ahci_ceva.c b/drivers/ata/ahci_ceva.c
index 207649d..59de2ca 100644
--- a/drivers/ata/ahci_ceva.c
+++ b/drivers/ata/ahci_ceva.c
@@ -50,21 +50,6 @@
 #define PPCFG_PSS_EN   (1 << 29)
 #define PPCFG_ESDF_EN  (1 << 31)
 
-#define PP2C_CIBGMN0x0F
-#define PP2C_CIBGMX(0x25 << 8)
-#define PP2C_CIBGN (0x18 << 16)
-#define PP2C_CINMP (0x29 << 24)
-
-#define PP3C_CWBGMN0x04
-#define PP3C_CWBGMX(0x0B << 8)
-#define PP3C_CWBGN (0x08 << 16)
-#define PP3C_CWNMP (0x0F << 24)
-
-#define PP4C_BMX   0x0a
-#define PP4C_BNM   (0x08 << 8)
-#define PP4C_SFD   (0x4a << 16)
-#define PP4C_PTST  (0x06 << 24)
-
 #define PP5C_RIT   0x60216
 #define PP5C_RCT   (0x7f0 << 20)
 
@@ -87,6 +72,11 @@
 
 struct ceva_ahci_priv {
 

RE: [PATCH v2] dmaengine: vdma: Add 64 bit addressing support to the driver

2015-09-23 Thread Anurag Kumar Vulisha
Hi Vinod,

Thanks for reviewing the patch

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Monday, September 21, 2015 9:27 PM
> To: Anurag Kumar Vulisha
> Cc: Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar Gala; Michal
> Simek; Soren Brinkmann; Dan Williams; afaer...@suse.de; Maxime Ripard;
> Laurent Pinchart; Appana Durga Kedareswara Rao; Anirudha Sarangi; Srikanth
> Vemula; devicetree@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-ker...@vger.kernel.org; dmaeng...@vger.kernel.org; Anurag Kumar
> Vulisha
> Subject: Re: [PATCH v2] dmaengine: vdma: Add 64 bit addressing support to
> the driver
>
> On Thu, Aug 27, 2015 at 09:19:18PM +0530, Anurag Kumar Vulisha wrote:
> > This VDMA  is a soft ip, which can be programmed to support
> > 32 bit addressing or greater than 32 bit addressing.
> >
> > When the VDMA ip is configured for 32 bit address space the transfer
> > start address is specified by a single register.
>
> would be good to specfiy which one
>

Will change  this in v3

> > When the  VDMA core is configured for an address space greater than 32
> > then each start address is specified by a combination of two
> > registers. The first register specifies the LSB 32 bits of address,
> > while the next register specifies the MSB 32 bits of address.For
> > example,5Ch will specify the LSB 32 bits while 60h will specify the
> > MSB 32 bits of the first start address.So we need to program two
> > registers at a time.
>
> can we have spaces after full stops and commas!
>

Will take care of this in v3 patch.

> > +/* Since vdma driver is trying to write to a register offset which is
> > +not a
> > + * multiple of 64 bits(ex : 0x5c), we are writing as two separate 32
> > +bits
> > + * instead of a single 64 bit register write.
> > + */
>
> This is not kernel style for multi-lines, pls refer to
> Documentation/CodingStyle
>

Will address this in v3 patch

> > +
> > +static inline void vdma_desc_write_64(struct xilinx_vdma_chan *chan,
> u32 reg,
> > +u32 value_lsb, u32 value_msb)
> > +{
> > +   /* Write the lsb 32 bits*/
> > +   writel(value_lsb, chan->xdev->regs + chan->desc_offset + reg);
> > +
> > +   /* Write the msb 32 bits */
> > +   writel(value_msb, chan->xdev->regs + chan->desc_offset + reg + 4);
>
> why not writeq

We are trying to write at a register address(ex:0x5c) which is not aligned on 8 
bytes  boundary.So if I try to use 64 bit write on it,unalignment  fault is 
getting  generated.To avoid that we are using two separate 32  bit writes.
We had this discussion in previous versions of this patch with Laurent Pinchart 
.I have also added this exaplanation in the comments above this function.

>
> > +   err = of_property_read_u32(node, "xlnx,addrwidth", &addr_width);
> > +
> > +   if (err < 0) {
> > +   /* Setting addr_width property to default 32 bits */
> > +   addr_width = 32;
> > +   }
>
> braces for a single line statement! Also space is redandant before if 
> condition
>

Will take care of this in v3 patch

Thanks,
Anurag Kumar V

> --
> ~Vinod


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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] dmaengine: vdma: Add 64 bit addressing support to the driver

2015-08-27 Thread Anurag Kumar Vulisha
This VDMA  is a soft ip, which can be programmed to support
32 bit addressing or greater than 32 bit addressing.

When the VDMA ip is configured for 32 bit address space the
transfer start address is specified by a single register.

When the  VDMA core is configured for an address space greater
than 32 then each start address is specified by a combination
of two registers. The first register specifies the LSB 32 bits
of address, while the next register specifies the MSB 32 bits
of address.For example,5Ch will specify the LSB 32 bits while
60h will specify the MSB 32 bits of the first start address.So
we need to program two registers at a time.

This patch adds the 64 bit addressing support to the vdma driver.

Signed-off-by: Anurag Kumar Vulisha 
---
  Changes in v2:
  1. Added dma-ranges property in device tree as suggested by Arnd Bergmann.
  2. Added device tree property(xlnx,addrwidth) for an identification of whether
 the IP block itself is configured in 64-bit or 32-bit mode as suggested by
 Laurent Pinchart.
  3. Modified the driver code based on the xlnx,addrwidth.
---
 .../devicetree/bindings/dma/xilinx/xilinx_vdma.txt |4 +
 drivers/dma/Kconfig|2 +-
 drivers/dma/xilinx/xilinx_vdma.c   |   81 +---
 3 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt 
b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
index e4c4d47..434d380 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
@@ -8,6 +8,8 @@ Required properties:
 - #dma-cells: Should be <1>, see "dmas" property below
 - reg: Should contain VDMA registers location and length.
 - xlnx,num-fstores: Should be the number of framebuffers as configured in h/w.
+- xlnx,addrwidth: Should be the vdma addressing size in bits(ex: 32 bits).
+- dma-ranges: Should be as the following 
 - dma-channel child node: Should have at least one channel and can have up to
two channels per device. This node specifies the properties of each
DMA channel (see child node properties below).
@@ -41,8 +43,10 @@ axi_vdma_0: axivdma@4003 {
compatible = "xlnx,axi-vdma-1.00.a";
#dma_cells = <1>;
reg = < 0x4003 0x1 >;
+   dma-ranges = <0x 0x 0x4000>;
xlnx,num-fstores = <0x8>;
xlnx,flush-fsync = <0x1>;
+   xlnx,addrwidth = <0x20>;
dma-channel@4003 {
compatible = "xlnx,axi-vdma-mm2s-channel";
interrupts = < 0 54 4 >;
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index bda2cb0..a7cd0a8 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -398,7 +398,7 @@ config FSL_EDMA
 
 config XILINX_VDMA
tristate "Xilinx AXI VDMA Engine"
-   depends on (ARCH_ZYNQ || MICROBLAZE)
+   depends on (ARCH_ZYNQ || MICROBLAZE || ARM64)
select DMA_ENGINE
help
  Enable support for Xilinx AXI VDMA Soft IP.
diff --git a/drivers/dma/xilinx/xilinx_vdma.c b/drivers/dma/xilinx/xilinx_vdma.c
index d8434d4..d4eebc9 100644
--- a/drivers/dma/xilinx/xilinx_vdma.c
+++ b/drivers/dma/xilinx/xilinx_vdma.c
@@ -98,6 +98,8 @@
 #define XILINX_VDMA_FRMDLY_STRIDE_FRMDLY_SHIFT 24
 #define XILINX_VDMA_FRMDLY_STRIDE_STRIDE_SHIFT 0
 
+#define XILINX_VDMA_REG_START_ADDRESS_64(n)(0x000c + 8 * (n))
+
 #define XILINX_VDMA_REG_START_ADDRESS(n)   (0x000c + 4 * (n))
 
 /* HW specific definitions */
@@ -143,16 +145,18 @@
  * @next_desc: Next Descriptor Pointer @0x00
  * @pad1: Reserved @0x04
  * @buf_addr: Buffer address @0x08
- * @pad2: Reserved @0x0C
- * @vsize: Vertical Size @0x10
- * @hsize: Horizontal Size @0x14
+ * @buf_addr_msb: Buffer msb address @0x0c
+ * @pad2: Reserved @0x10
+ * @vsize: Vertical Size @0x14
+ * @hsize: Horizontal Size @0x18
  * @stride: Number of bytes between the first
- * pixels of each horizontal line @0x18
+ * pixels of each horizontal line @0x1C
  */
 struct xilinx_vdma_desc_hw {
u32 next_desc;
u32 pad1;
u32 buf_addr;
+   u32 buf_addr_msb;
u32 pad2;
u32 vsize;
u32 hsize;
@@ -206,6 +210,7 @@ struct xilinx_vdma_tx_descriptor {
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
+ * @ext_addr: Indicates 64 bit addressing is supported by dma channel
  */
 struct xilinx_vdma_chan {
struct xilinx_vdma_device *xdev;
@@ -229,6 +234,7 @@ struct xilinx_vdma_chan {
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
+   bool ext_addr;
 };
 
 /**
@@ -239,6 +245,7 @@ struct xilinx_vdma_chan {
  * @chan: Driver specific VDMA channel
  * @has_sg: Specifies whether Scatter-Gather is

[PATCH v2] i2c: removed work arounds in i2c driver for Zynq Ultrascale+ MPSoC

2015-07-10 Thread Anurag Kumar Vulisha
Cadence 1.0 version has bugs which have been fixed in the cadence 1.4 version.
This patch removes the quirks present in the driver for cadence 1.4 version.

Signed-off-by: Anurag Kumar Vulisha 
---
 Changes in v2:
1.Changed the commit message.
2.Changed the compatible string description in devicetree binding 
Documentation.
3.Used BIT(0) in macro and moved "};" to next line in r1p10_i2c_def 
platform data. 
---
 .../devicetree/bindings/i2c/i2c-cadence.txt|6 ++-
 drivers/i2c/busses/i2c-cadence.c   |   68 ---
 2 files changed, 62 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt 
b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
index 7cb0b56..ebaa90c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
@@ -2,7 +2,11 @@ Binding for the Cadence I2C controller
 
 Required properties:
   - reg: Physical base address and size of the controller's register area.
-  - compatible: Compatibility string. Must be 'cdns,i2c-r1p10'.
+  - compatible: Should contain one of:
+   * "cdns,i2c-r1p10"
+   Note:   Use this when cadence i2c controller version 1.0 is 
used.
+   * "cdns,i2c-r1p14"
+   Note:   Use this when cadence i2c controller version 1.4 is 
used.
   - clocks: Input clock specifier. Refer to common clock bindings.
   - interrupts: Interrupt specifier. Refer to interrupt bindings.
   - #address-cells: Should be 1.
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 2ee78e0..5df288b 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Register offsets for the I2C device. */
 #define CDNS_I2C_CR_OFFSET 0x00 /* Control Register, RW */
@@ -113,6 +114,8 @@
 
 #define CDNS_I2C_TIMEOUT_MAX   0xFF
 
+#define CDNS_I2C_BROKEN_HOLD_BIT   BIT(0)
+
 #define cdns_i2c_readreg(offset)   readl_relaxed(id->membase + offset)
 #define cdns_i2c_writereg(val, offset) writel_relaxed(val, id->membase + 
offset)
 
@@ -135,6 +138,7 @@
  * @bus_hold_flag: Flag used in repeated start for clearing HOLD bit
  * @clk:   Pointer to struct clk
  * @clk_rate_change_nb:Notifier block for clock rate changes
+ * @quirks:flag for broken hold bit usage in r1p10
  */
 struct cdns_i2c {
void __iomem *membase;
@@ -154,6 +158,11 @@ struct cdns_i2c {
unsigned int bus_hold_flag;
struct clk *clk;
struct notifier_block clk_rate_change_nb;
+   u32 quirks;
+};
+
+struct cdns_platform_data {
+   u32 quirks;
 };
 
 #define to_cdns_i2c(_nb)   container_of(_nb, struct cdns_i2c, \
@@ -172,6 +181,12 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
cdns_i2c_writereg(reg & ~CDNS_I2C_CR_HOLD, CDNS_I2C_CR_OFFSET);
 }
 
+static inline bool cdns_is_holdquirk(struct cdns_i2c *id, bool hold_wrkaround)
+{
+   return (hold_wrkaround &&
+   (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1));
+}
+
 /**
  * cdns_i2c_isr - Interrupt handler for the I2C device
  * @irq:   irq number for the I2C device
@@ -186,6 +201,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
 {
unsigned int isr_status, avail_bytes, updatetx;
unsigned int bytes_to_send;
+   bool hold_quirk;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
@@ -208,6 +224,8 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
if (id->recv_count > id->curr_recv_count)
updatetx = 1;
 
+   hold_quirk = (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) && updatetx;
+
/* When receiving, handle data interrupt and completion interrupt */
if (id->p_recv_buf &&
((isr_status & CDNS_I2C_IXR_COMP) ||
@@ -229,8 +247,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
id->recv_count--;
id->curr_recv_count--;
 
-   if (updatetx &&
-   (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
+   if (cdns_is_holdquirk(id, hold_quirk))
break;
}
 
@@ -241,8 +258,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
 * maintain transfer size non-zero while performing a large
 * receive operation.
 */
-   if (updatetx &&
-   (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
+   if (cdns_is_holdquirk(id, hold_quirk)) {
/* wait while fifo is full */
while 

RE: [PATCH] i2c: removed work arounds in i2c driver for Zynq Ultrascale+ MPSoC

2015-07-10 Thread Anurag Kumar Vulisha

Hi,
Thanks for reviewing the code. I will make the changes as said and send 
the patch as v2.
Thanks,
Anurag Kumar V

> -Original Message-
> From: Wolfram Sang [mailto:w...@the-dreams.de]
> Sent: Friday, July 10, 2015 1:39 AM
> To: Anurag Kumar Vulisha
> Cc: robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; Michal Simek; Soren
> Brinkmann; linux-...@vger.kernel.org; devicetree@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Punnaiah
> Choudary Kalluri; Harini Katakam; Anirudha Sarangi; Srikanth Vemula; Anurag
> Kumar Vulisha
> Subject: Re: [PATCH] i2c: removed work arounds in i2c driver for Zynq
> Ultrascale+ MPSoC
>
> Hi,
>
> thanks for the submission!
>
> > +#define CDNS_I2C_BROKEN_HOLD_BIT   0x0001
>
> BIT(0) maybe?
>
> > +static const struct cdns_platform_data r1p10_i2c_def = {
> > +   .quirks = CDNS_I2C_BROKEN_HOLD_BIT, };
>
> Closing '}' should be on seperate line.
>
> And what Mark said.
>
> Other than that, looks okay I'd say.



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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] i2c: removed work arounds in i2c driver for Zynq Ultrascale+ MPSoC

2015-06-18 Thread Anurag Kumar Vulisha
Removed the workarounds present in the cadence i2c driver for
Zynq Ultrascale+ MPSoC

Signed-off-by: Anurag Kumar Vulisha 
---
 .../devicetree/bindings/i2c/i2c-cadence.txt|2 +-
 drivers/i2c/busses/i2c-cadence.c   |   67 ---
 2 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt 
b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
index 7cb0b56..11ef7f2 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-cadence.txt
@@ -2,7 +2,7 @@ Binding for the Cadence I2C controller
 
 Required properties:
   - reg: Physical base address and size of the controller's register area.
-  - compatible: Compatibility string. Must be 'cdns,i2c-r1p10'.
+  - compatible: Compatibility string.Use 'cdns,i2c-r1p10' or 'cdns,i2c-r1p14'
   - clocks: Input clock specifier. Refer to common clock bindings.
   - interrupts: Interrupt specifier. Refer to interrupt bindings.
   - #address-cells: Should be 1.
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 2ee78e0..6ad1571 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Register offsets for the I2C device. */
 #define CDNS_I2C_CR_OFFSET 0x00 /* Control Register, RW */
@@ -113,6 +114,8 @@
 
 #define CDNS_I2C_TIMEOUT_MAX   0xFF
 
+#define CDNS_I2C_BROKEN_HOLD_BIT   0x0001
+
 #define cdns_i2c_readreg(offset)   readl_relaxed(id->membase + offset)
 #define cdns_i2c_writereg(val, offset) writel_relaxed(val, id->membase + 
offset)
 
@@ -135,6 +138,7 @@
  * @bus_hold_flag: Flag used in repeated start for clearing HOLD bit
  * @clk:   Pointer to struct clk
  * @clk_rate_change_nb:Notifier block for clock rate changes
+ * @quirks:flag for broken hold bit usage in r1p10
  */
 struct cdns_i2c {
void __iomem *membase;
@@ -154,6 +158,11 @@ struct cdns_i2c {
unsigned int bus_hold_flag;
struct clk *clk;
struct notifier_block clk_rate_change_nb;
+   u32 quirks;
+};
+
+struct cdns_platform_data {
+   u32 quirks;
 };
 
 #define to_cdns_i2c(_nb)   container_of(_nb, struct cdns_i2c, \
@@ -172,6 +181,12 @@ static void cdns_i2c_clear_bus_hold(struct cdns_i2c *id)
cdns_i2c_writereg(reg & ~CDNS_I2C_CR_HOLD, CDNS_I2C_CR_OFFSET);
 }
 
+static inline bool cdns_is_holdquirk(struct cdns_i2c *id, bool hold_wrkaround)
+{
+   return (hold_wrkaround &&
+   (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1));
+}
+
 /**
  * cdns_i2c_isr - Interrupt handler for the I2C device
  * @irq:   irq number for the I2C device
@@ -186,6 +201,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
 {
unsigned int isr_status, avail_bytes, updatetx;
unsigned int bytes_to_send;
+   bool hold_quirk;
struct cdns_i2c *id = ptr;
/* Signal completion only after everything is updated */
int done_flag = 0;
@@ -208,6 +224,8 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
if (id->recv_count > id->curr_recv_count)
updatetx = 1;
 
+   hold_quirk = (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) && updatetx;
+
/* When receiving, handle data interrupt and completion interrupt */
if (id->p_recv_buf &&
((isr_status & CDNS_I2C_IXR_COMP) ||
@@ -229,8 +247,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
id->recv_count--;
id->curr_recv_count--;
 
-   if (updatetx &&
-   (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1))
+   if (cdns_is_holdquirk(id, hold_quirk))
break;
}
 
@@ -241,8 +258,7 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
 * maintain transfer size non-zero while performing a large
 * receive operation.
 */
-   if (updatetx &&
-   (id->curr_recv_count == CDNS_I2C_FIFO_DEPTH + 1)) {
+   if (cdns_is_holdquirk(id, hold_quirk)) {
/* wait while fifo is full */
while (cdns_i2c_readreg(CDNS_I2C_XFER_SIZE_OFFSET) !=
   (id->curr_recv_count - CDNS_I2C_FIFO_DEPTH))
@@ -264,6 +280,22 @@ static irqreturn_t cdns_i2c_isr(int irq, void *ptr)
  CDNS_I2C_XFER_SIZE_OFFSET);
id->curr_recv_count = id->recv_count;
}
+   } else if (id->recv_count && !hold_quirk &&
+   !id

[PATCH v2] gpio: Added GPIO support to Zynq Ultrascale+ MPSoC

2015-06-04 Thread Anurag Kumar Vulisha
Added support to Zynq Ultrascale+ MPSoC on the existing zynq gpio driver.

Signed-off-by: Anurag Kumar Vulisha 
---
 Chnages in v2:
1.Added device tree bingings for Zynq Ultrascale+ MPSoC
2.Changed the commit message and subject from ZynqMP to
  Zynq Ultrascale+ MPSoC
---
 .../devicetree/bindings/gpio/gpio-zynq.txt |2 +-
 drivers/gpio/Kconfig   |2 +-
 drivers/gpio/gpio-zynq.c   |  191 +---
 3 files changed, 127 insertions(+), 68 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-zynq.txt 
b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
index 986371a..db4c6a6 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-zynq.txt
@@ -6,7 +6,7 @@ Required properties:
  - First cell is the GPIO line number
  - Second cell is used to specify optional
parameters (unused)
-- compatible   : Should be "xlnx,zynq-gpio-1.0"
+- compatible   : Should be "xlnx,zynq-gpio-1.0" or 
"xlnx,zynqmp-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
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index caefe80..eef3a74 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -505,7 +505,7 @@ config GPIO_ZEVIO
 
 config GPIO_ZYNQ
tristate "Xilinx Zynq GPIO support"
-   depends on ARCH_ZYNQ
+   depends on ARCH_ZYNQ || ARCH_ZYNQMP
select GPIOLIB_IRQCHIP
help
  Say yes here to support Xilinx Zynq GPIO controller.
diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
index 184c4b1..d9d26aa 100644
--- a/drivers/gpio/gpio-zynq.c
+++ b/drivers/gpio/gpio-zynq.c
@@ -18,34 +18,47 @@
 #include 
 #include 
 #include 
+#include 
 
 #define DRIVER_NAME "zynq-gpio"
 
 /* Maximum banks */
 #define ZYNQ_GPIO_MAX_BANK 4
+#define ZYNQMP_GPIO_MAX_BANK   6
 
 #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)
+#define ZYNQMP_GPIO_BANK0_NGPIO 26
+#define ZYNQMP_GPIO_BANK1_NGPIO 26
+#define ZYNQMP_GPIO_BANK2_NGPIO 26
+#define ZYNQMP_GPIO_BANK3_NGPIO 32
+#define ZYNQMP_GPIO_BANK4_NGPIO 32
+#define ZYNQMP_GPIO_BANK5_NGPIO 32
+
+#defineZYNQ_GPIO_NR_GPIOS  118
+#defineZYNQMP_GPIO_NR_GPIOS174
+
+#define ZYNQ_GPIO_BANK0_PIN_MIN(str)   0
+#define ZYNQ_GPIO_BANK0_PIN_MAX(str)   (ZYNQ_GPIO_BANK0_PIN_MIN(str) + \
+   ZYNQ##str##_GPIO_BANK0_NGPIO - 1)
+#define ZYNQ_GPIO_BANK1_PIN_MIN(str)   (ZYNQ_GPIO_BANK0_PIN_MAX(str) + 1)
+#define ZYNQ_GPIO_BANK1_PIN_MAX(str)   (ZYNQ_GPIO_BANK1_PIN_MIN(str) + \
+   ZYNQ##str##_GPIO_BANK1_NGPIO - 1)
+#define ZYNQ_GPIO_BANK2_PIN_MIN(str)   (ZYNQ_GPIO_BANK1_PIN_MAX(str) + 1)
+#define ZYNQ_GPIO_BANK2_PIN_MAX(str)   (ZYNQ_GPIO_BANK2_PIN_MIN(str) + \
+   ZYNQ##str##_GPIO_BANK2_NGPIO - 1)
+#define ZYNQ_GPIO_BANK3_PIN_MIN(str)   (ZYNQ_GPIO_BANK2_PIN_MAX(str) + 1)
+#define ZYNQ_GPIO_BANK3_PIN_MAX(str)   (ZYNQ_GPIO_BANK3_PIN_MIN(str) + \
+   ZYNQ##str##_GPIO_BANK3_NGPIO - 1)
+#define ZYNQ_GPIO_BANK4_PIN_MIN(str)   (ZYNQ_GPIO_BANK3_PIN_MAX(str) + 1)
+#define ZYNQ_GPIO_BANK4_PIN_MAX(str)   (ZYNQ_GPIO_BANK4_PIN_MIN(str) + \
+   ZYNQ##str##_GPIO_BANK4_NGPIO - 1)
+#define ZYNQ_GPIO_BANK5_PIN_MIN(str)   (ZYNQ_GPIO_BANK4_PIN_MAX(str) + 1)
+#define ZYNQ_GPIO_BANK5_PIN_MAX(str)   (ZYNQ_GPIO_BANK5_PIN_MIN(str) + \
+   ZYNQ##str##_GPIO_BANK