[PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Andy Shevchenko]

Intel Quark UART uses DesignWare DMA IP. Though the DMA IP is connected in such
way that handshake interface uses inverted polarity. We have to provide a
possibility to set this in the DMA driver when configuring a channel.

Introduce a new member of custom slave configuration called 'polarity' and set
active low polarity in case this value is 'true'.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c                | 2 ++
 include/linux/platform_data/dma-dw.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 81b06df..9c7bc7a 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -150,6 +150,8 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
 	cfghi |= DWC_CFGH_DST_PER(dwc->dws.dst_id);
 	cfghi |= DWC_CFGH_SRC_PER(dwc->dws.src_id);
 
+	cfglo |= dwc->dws.polarity ? DWC_CFGL_HS_DST_POL | DWC_CFGL_HS_SRC_POL : 0;
+
 	channel_writel(dwc, CFG_LO, cfglo);
 	channel_writel(dwc, CFG_HI, cfghi);
 
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index d15d8ba..192f3a2 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -23,6 +23,7 @@
  * @dst_id:	dst request line
  * @m_master:	memory master for transfers on allocated channel
  * @p_master:	peripheral master for transfers on allocated channel
+ * @polarity:	set active low polarity of handshake interface
  */
 struct dw_dma_slave {
 	struct device		*dma_dev;
@@ -30,6 +31,7 @@ struct dw_dma_slave {
 	u8			dst_id;
 	u8			m_master;
 	u8			p_master;
+	bool			polarity;
 };
 
 /**
-- 
2.8.0.rc3

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Bryan O'Donoghue]

On Wed, 2016-04-27 at 16:48 +0300, Andy Shevchenko wrote:
> +       bool                    polarity;

So this variable is not very intuitively named.

You end up setting somepointer->polarity = true; in a later patch. 

Since you're respining a V4 I'd suggest a name that describes a little
bit better than polarity. Setting polarity = true is a little bit liked
being asked "you you like ice-cream or apple pie" and then saying "yes
please".

---
bod

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Andy Shevchenko]

On Thu, 2016-05-05 at 18:54 +0100, Bryan O'Donoghue wrote:
> On Wed, 2016-04-27 at 16:48 +0300, Andy Shevchenko wrote:
> > 
> > +       bool                    polarity;
> So this variable is not very intuitively named.

There is a help above. This is a property of the Synopsys DesignWare DMA
engine. Anyone familiar with datasheet easily understands this.

> 
> You end up setting somepointer->polarity = true; in a later patch. 
> 
> Since you're respining a V4 I'd suggest a name that describes a little
> bit better than polarity. Setting polarity = true is a little bit
> liked
> being asked "you you like ice-cream or apple pie" and then saying "yes
> please".

It's about handshake interface polarity, so, what about hs_polarity?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Bryan O'Donoghue]

On Fri, 2016-05-06 at 13:42 +0300, Andy Shevchenko wrote:

> It's about handshake interface polarity, so, what about hs_polarity?

Works for me.

---
bod

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Maarten Brock]

----- Original Message -----
From: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]
To: Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie], Peter Hurley [mailto:peter@hurleysoftware.com], linux-serial@vger.kernel.org, Vinod Koul [mailto:vinod.koul@intel.com], linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org, Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org], ismo.puustinen@intel.com, Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
Sent: Fri, 06 May 2016 12:42:00 +0200
Subject: Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface


> On Thu, 2016-05-05 at 18:54 +0100, Bryan O'Donoghue wrote:
> > On Wed, 2016-04-27 at 16:48 +0300, Andy Shevchenko wrote:
> > > 
> > > +       bool                    polarity;
> > So this variable is not very intuitively named.
> 
> There is a help above. This is a property of the Synopsys DesignWare DMA
> engine. Anyone familiar with datasheet easily understands this.
> 
> > 
> > You end up setting somepointer->polarity = true; in a later patch. 
> > 
> > Since you're respining a V4 I'd suggest a name that describes a little
> > bit better than polarity. Setting polarity = true is a little bit
> > liked being asked "you you like ice-cream or apple pie" and then
> > saying "yes please".
> 
> It's about handshake interface polarity, so, what about hs_polarity?

So it means: handshake has polarity (true) or handshake has no polarity
(omnidirectional?) (false), right?

Maarten

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Andy Shevchenko]

On Fri, May 6, 2016 at 2:20 PM, Maarten Brock <m.brock@vanmierlo.com> wrote:
>> > >
>> > > +       bool                    polarity;
>> > So this variable is not very intuitively named.
>>
>> There is a help above. This is a property of the Synopsys DesignWare DMA
>> engine. Anyone familiar with datasheet easily understands this.
>>
>> >
>> > You end up setting somepointer->polarity = true; in a later patch.
>> >
>> > Since you're respining a V4 I'd suggest a name that describes a little
>> > bit better than polarity. Setting polarity = true is a little bit
>> > liked being asked "you you like ice-cream or apple pie" and then
>> > saying "yes please".
>>
>> It's about handshake interface polarity, so, what about hs_polarity?
>
> So it means: handshake has polarity (true) or handshake has no polarity
> (omnidirectional?) (false), right?

It means that handshake polarity _signal_ is inverted (true) or
default (false) in terms of hardware gates.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Maarten Brock]

----- Original Message -----
From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
To: Maarten Brock [mailto:m.brock@vanmierlo.com]
Cc: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com], Bryan O'Donoghue [mailto:pure.logic@nexus-software.ie], Peter Hurley [mailto:peter@hurleysoftware.com], linux-serial@vger.kernel.org [mailto:linux-serial@vger.kernel.org], Vinod Koul [mailto:vinod.koul@intel.com], linux-kernel@vger.kernel.org [mailto:linux-kernel@vger.kernel.org], dmaengine [mailto:dmaengine@vger.kernel.org], Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org], Puustinen, Ismo [mailto:ismo.puustinen@intel.com], Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
Sent: Fri, 06 May 2016 13:38:44 +0200
Subject: Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface


> On Fri, May 6, 2016 at 2:20 PM, Maarten Brock <m.brock@vanmierlo.com> wrote:
> >> > >
> >> > > +       bool                    polarity;
> >> > So this variable is not very intuitively named.
> >>
> >> There is a help above. This is a property of the Synopsys DesignWare DMA
> >> engine. Anyone familiar with datasheet easily understands this.
> >>
> >> >
> >> > You end up setting somepointer->polarity = true; in a later patch.
> >> >
> >> > Since you're respining a V4 I'd suggest a name that describes a little
> >> > bit better than polarity. Setting polarity = true is a little bit
> >> > liked being asked "you you like ice-cream or apple pie" and then
> >> > saying "yes please".
> >>
> >> It's about handshake interface polarity, so, what about hs_polarity?
> >
> > So it means: handshake has polarity (true) or handshake has no polarity
> > (omnidirectional?) (false), right?
> 
> It means that handshake polarity _signal_ is inverted (true) or
> default (false) in terms of hardware gates.

Would it not be better then to name it handshake_inverted ? That is
something you can ask and answer with true or false.

Maarten

> -- 
> With Best Regards,
> Andy Shevchenko
> 

Re: [PATCH v3 03/11] dmaengine: dw: set polarity of handshake interface

[Andy Shevchenko]

On Fri, 2016-05-06 at 17:48 +0200, Maarten Brock wrote:

> +       bool                    polarity;
> > > > > So this variable is not very intuitively named.
> > > > There is a help above. This is a property of the Synopsys
> > > > DesignWare DMA
> > > > engine. Anyone familiar with datasheet easily understands this.
> > > > 
> > > > > 
> > > > > 
> > > > > You end up setting somepointer->polarity = true; in a later
> > > > > patch.
> > > > > 
> > > > > Since you're respining a V4 I'd suggest a name that describes
> > > > > a little
> > > > > bit better than polarity. Setting polarity = true is a little
> > > > > bit
> > > > > liked being asked "you you like ice-cream or apple pie" and
> > > > > then
> > > > > saying "yes please".
> > > > It's about handshake interface polarity, so, what about
> > > > hs_polarity?
> > > So it means: handshake has polarity (true) or handshake has no
> > > polarity
> > > (omnidirectional?) (false), right?
> > It means that handshake polarity _signal_ is inverted (true) or
> > default (false) in terms of hardware gates.
> Would it not be better then to name it handshake_inverted ? That is
> something you can ask and answer with true or false.
> 

I would stick for now with hs_polarity as agreed with Bryan. Without
specification any (more or less short) name will suck anyway.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy