Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-11 Thread Ivan T. Ivanov
Hi, On Mon, 2014-02-10 at 12:29 -0800, Courtney Cavin wrote: On Mon, Feb 10, 2014 at 08:41:44PM +0100, Ivan T. Ivanov wrote: Hi, On Mon, 2014-02-10 at 11:47 -0600, Andy Gross wrote: On Mon, Feb 10, 2014 at 06:55:02PM +0200, Ivan T. Ivanov wrote: [] Bail

Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Ivan T. Ivanov
Hi Daniel, On Fri, 2014-02-07 at 16:34 +, dsned...@codeaurora.org wrote: From: Ivan T. Ivanov iiva...@mm-sol.com snip + +static int spi_qup_set_state(struct spi_qup *controller, u32 state) +{ + unsigned long loop = 0; + u32 cur_state; + + cur_state =

Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread dsneddon
Hi Ivan, + +static int spi_qup_set_state(struct spi_qup *controller, u32 state) +{ + unsigned long loop = 0; + u32 cur_state; + + cur_state = readl_relaxed(controller-base + QUP_STATE); Make sure the state is valid before you read the current state. Why?

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Ivan T. Ivanov
Hi, On Fri, 2014-02-07 at 17:12 +, Mark Brown wrote: On Thu, Feb 06, 2014 at 06:57:48PM +0200, Ivan T. Ivanov wrote: This looks mostly good, there's a few odd things and missing use of framework features. Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Ivan T. Ivanov
Hi Andy, On Fri, 2014-02-07 at 11:32 -0600, Andy Gross wrote: On Fri, Feb 07, 2014 at 11:52:33AM +0200, Ivan T. Ivanov wrote: snip +static int spi_qup_transfer_do(struct spi_qup *controller, + struct spi_qup_device *chip, +

Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Ivan T. Ivanov
Hi Dan, On Mon, 2014-02-10 at 16:21 +, dsned...@codeaurora.org wrote: Hi Ivan, + +static int spi_qup_set_state(struct spi_qup *controller, u32 state) +{ + unsigned long loop = 0; + u32 cur_state; + + cur_state = readl_relaxed(controller-base +

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Mark Brown
On Mon, Feb 10, 2014 at 06:29:26PM +0200, Ivan T. Ivanov wrote: On Fri, 2014-02-07 at 17:12 +, Mark Brown wrote: + if (!xfer) + return IRQ_HANDLED; Are you sure? It seems wrong to just ignore interrupts, some comments would help explain why. Of course this should never

Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread dsneddon
Hi Ivan, +static int spi_qup_set_state(struct spi_qup *controller, u32 state) +{ + unsigned long loop = 0; + u32 cur_state; + + cur_state = readl_relaxed(controller-base + QUP_STATE); Make sure the state is valid before you read the current state. Why?

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Andy Gross
On Mon, Feb 10, 2014 at 06:55:02PM +0200, Ivan T. Ivanov wrote: [] Bail here? I don't know. What will be the consequences if controller continue to operate on its default rate? It is unclear. But if you can't set the rate that is configured or if there is a

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Ivan T. Ivanov
Hi, On Mon, 2014-02-10 at 11:47 -0600, Andy Gross wrote: On Mon, Feb 10, 2014 at 06:55:02PM +0200, Ivan T. Ivanov wrote: [] Bail here? I don't know. What will be the consequences if controller continue to operate on its default rate? It is unclear. But if

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Courtney Cavin
On Mon, Feb 10, 2014 at 08:41:44PM +0100, Ivan T. Ivanov wrote: Hi, On Mon, 2014-02-10 at 11:47 -0600, Andy Gross wrote: On Mon, Feb 10, 2014 at 06:55:02PM +0200, Ivan T. Ivanov wrote: [] Bail here? I don't know. What will be the consequences if controller

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Ivan T. Ivanov
Hi, On Mon, 2014-02-10 at 12:29 -0800, Courtney Cavin wrote: On Mon, Feb 10, 2014 at 08:41:44PM +0100, Ivan T. Ivanov wrote: Hi, On Mon, 2014-02-10 at 11:47 -0600, Andy Gross wrote: On Mon, Feb 10, 2014 at 06:55:02PM +0200, Ivan T. Ivanov wrote: [] Bail here?

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-10 Thread Mark Brown
On Mon, Feb 10, 2014 at 10:59:54PM +0200, Ivan T. Ivanov wrote: On Mon, 2014-02-10 at 12:29 -0800, Courtney Cavin wrote: A developer doesn't have to have much skill at all to copy-paste DT configurations around and muck with numbers I agree with Andy here, early validation is a good

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Ivan T. Ivanov
Hi Andy, On Fri, 2014-02-07 at 01:39 -0600, Andy Gross wrote: On Thu, Feb 06, 2014 at 06:57:48PM +0200, Ivan T. Ivanov wrote: From: Ivan T. Ivanov iiva...@mm-sol.com Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a common data path (an output FIFO and an input

Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread dsneddon
From: Ivan T. Ivanov iiva...@mm-sol.com Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a common data path (an output FIFO and an input FIFO) for serial peripheral interface (SPI) mini-core. SPI in master mode support up to 50MHz, up to four chip selects, and a

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Josh Cartwright
On Fri, Feb 07, 2014 at 01:39:52AM -0600, Andy Gross wrote: On Thu, Feb 06, 2014 at 06:57:48PM +0200, Ivan T. Ivanov wrote: From: Ivan T. Ivanov iiva...@mm-sol.com Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a common data path (an output FIFO and an input FIFO)

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Mark Brown
On Thu, Feb 06, 2014 at 06:57:48PM +0200, Ivan T. Ivanov wrote: This looks mostly good, there's a few odd things and missing use of framework features. Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a common data path (an output FIFO and an input FIFO) for serial

Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Mark Brown
On Fri, Feb 07, 2014 at 04:34:25PM -, dsned...@codeaurora.org wrote: From: Ivan T. Ivanov iiva...@mm-sol.com Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a common data path (an output FIFO and an input FIFO) Folks, please remember to delete irrelevant context

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Mark Brown
On Fri, Feb 07, 2014 at 10:51:27AM -0600, Josh Cartwright wrote: config SPI_QUP tristate Qualcomm SPI Support with QUP interface depends on OF depends on ARM Does this really depend on ARM? If so why? depends on ARCH_MSM_DT ||

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Mark Brown
On Fri, Feb 07, 2014 at 11:20:51AM -0600, Josh Cartwright wrote: On Fri, Feb 07, 2014 at 05:18:34PM +, Mark Brown wrote: On Fri, Feb 07, 2014 at 10:51:27AM -0600, Josh Cartwright wrote: config SPI_QUP tristate Qualcomm SPI Support with QUP interface depends

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Andy Gross
On Fri, Feb 07, 2014 at 11:52:33AM +0200, Ivan T. Ivanov wrote: Hi Andy, On Fri, 2014-02-07 at 01:39 -0600, Andy Gross wrote: On Thu, Feb 06, 2014 at 06:57:48PM +0200, Ivan T. Ivanov wrote: From: Ivan T. Ivanov iiva...@mm-sol.com Qualcomm Universal Peripheral (QUP) core is an

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Josh Cartwright
On Fri, Feb 07, 2014 at 05:31:08PM +, Mark Brown wrote: On Fri, Feb 07, 2014 at 11:20:51AM -0600, Josh Cartwright wrote: On Fri, Feb 07, 2014 at 05:18:34PM +, Mark Brown wrote: On Fri, Feb 07, 2014 at 10:51:27AM -0600, Josh Cartwright wrote: config SPI_QUP

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Mark Brown
On Fri, Feb 07, 2014 at 11:46:43AM -0600, Josh Cartwright wrote: On Fri, Feb 07, 2014 at 05:31:08PM +, Mark Brown wrote: That's not ARM only and I thought we were getting generic versions of it anyway? ARMv8, MIPS, Microblaze, Hexagon and SH also define it. Okay, that's fair. I'm

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Mark Brown
On Fri, Feb 07, 2014 at 11:32:07AM -0600, Andy Gross wrote: On Fri, Feb 07, 2014 at 11:52:33AM +0200, Ivan T. Ivanov wrote: To repeat what I said in my earlier e-mail please delete irrelevant context from your mails so any new content you are including is discoverable. Did you see any

Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-07 Thread Andy Gross
On Fri, Feb 07, 2014 at 05:52:34PM +, Mark Brown wrote: On Fri, Feb 07, 2014 at 11:32:07AM -0600, Andy Gross wrote: On Fri, Feb 07, 2014 at 11:52:33AM +0200, Ivan T. Ivanov wrote: [... snip ...] The platform doesn't have support for PM right now. So it's probably better to

[PATCH 2/2] spi: Add Qualcomm QUP SPI controller support

2014-02-06 Thread Ivan T. Ivanov
From: Ivan T. Ivanov iiva...@mm-sol.com Qualcomm Universal Peripheral (QUP) core is an AHB slave that provides a common data path (an output FIFO and an input FIFO) for serial peripheral interface (SPI) mini-core. SPI in master mode support up to 50MHz, up to four chip selects, and a programmable