[v2] spi/pxa2xx-pci: Add common clock framework support in PCI glue layer

Commit Message

Chew Chiau Ee July 24, 2014, 5:10 p.m. UTC

From: Chew, Chiau Ee <[e-mail]>

SPI PXA2XX core layer has dependency on common clock framework
to obtain information on host supported clock rate. Thus, we
setup the clock device in the PCI glue layer to enable PCI mode
host pass in the clock rate information.

Signed-off-by: Chew, Chiau Ee <[e-mail]>
Acked-by: Kweh, Hock Leong <[e-mail]>
---
 changelog v2:
 i. add CONFIG_COMMON_CLK dependency for SPI_PXA2XX_PCI in Kconfig
 ii. add clk cleanup in remove()

 drivers/spi/Kconfig          |    2 +-
 drivers/spi/spi-pxa2xx-pci.c |   21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

Comments

Mika Westerberg July 25, 2014, 5:32 a.m. UTC | #1

On Fri, Jul 25, 2014 at 01:10:54AM +0800, [e-mail] wrote:
> From: Chew, Chiau Ee <[e-mail]>
> 
> SPI PXA2XX core layer has dependency on common clock framework
> to obtain information on host supported clock rate. Thus, we
> setup the clock device in the PCI glue layer to enable PCI mode
> host pass in the clock rate information.
> 
> Signed-off-by: Chew, Chiau Ee <[e-mail]>
> Acked-by: Kweh, Hock Leong <[e-mail]>

Looks reasonable,

Acked-by: Mika Westerberg <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andrea Adami July 29, 2014, 4:13 p.m. UTC | #2

On Fri, Jul 25, 2014 at 7:32 AM, Mika Westerberg
<[e-mail]> wrote:
> On Fri, Jul 25, 2014 at 01:10:54AM +0800, [e-mail] wrote:
>> From: Chew, Chiau Ee <[e-mail]>
>>
>> SPI PXA2XX core layer has dependency on common clock framework
>> to obtain information on host supported clock rate. Thus, we
>> setup the clock device in the PCI glue layer to enable PCI mode
>> host pass in the clock rate information.
>>
>> Signed-off-by: Chew, Chiau Ee <[e-mail]>
>> Acked-by: Kweh, Hock Leong <[e-mail]>
>
> Looks reasonable,
>
> Acked-by: Mika Westerberg <[e-mail]>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi,

note that a previous version of the patch breaks spi on pxa machines:
tested on pxa250 poodle and pxa255 corgi.
The patch is included in linux-Yocto since 3.14.

The evident error is the absence of the touchscreen and on corgi a total crash.
pxa2xx-spi: probe of pxa2xx-spi.1 failed with error -2

I sent the full logs to the pxa maintainers.
Please let me know if there are any trivial fixes to apply on the top
of this patch for the non-pci, non-DT boards or better workarounds.

Thanks

Andrea
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Brown July 29, 2014, 6:55 p.m. UTC | #3

On Tue, Jul 29, 2014 at 06:13:06PM +0200, Andrea Adami wrote:

> note that a previous version of the patch breaks spi on pxa machines:
> tested on pxa250 poodle and pxa255 corgi.
> The patch is included in linux-Yocto since 3.14.

The interesting question is if this version works...

Andrea Adami July 29, 2014, 10:06 p.m. UTC | #4

On Tue, Jul 29, 2014 at 8:55 PM, Mark Brown <[e-mail]> wrote:
> On Tue, Jul 29, 2014 at 06:13:06PM +0200, Andrea Adami wrote:
>
>> note that a previous version of the patch breaks spi on pxa machines:
>> tested on pxa250 poodle and pxa255 corgi.
>> The patch is included in linux-Yocto since 3.14.
>
> The interesting question is if this version works...

Happy to report that I could finally test on poodle/pxa250 the current
v2 and the patch doesn't seem to disturb the probe.
Sorry for the noise.

Andrea
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Darren Hart Aug. 4, 2014, 3:55 p.m. UTC | #5

On 7/29/14, 15:06, "Andrea Adami" <[e-mail]> wrote:

>On Tue, Jul 29, 2014 at 8:55 PM, Mark Brown <[e-mail]> wrote:
>> On Tue, Jul 29, 2014 at 06:13:06PM +0200, Andrea Adami wrote:
>>
>>> note that a previous version of the patch breaks spi on pxa machines:
>>> tested on pxa250 poodle and pxa255 corgi.
>>> The patch is included in linux-Yocto since 3.14.
>>
>> The interesting question is if this version works...
>
>Happy to report that I could finally test on poodle/pxa250 the current
>v2 and the patch doesn't seem to disturb the probe.
>Sorry for the noise.
>
>Andrea
>

I tested this (v2) on the MinnowBoard-Max (Baytrail Atom E3825) in PCI
mode and successfully attached an MCP32XX ADC and tested it's IIO
interface.

Mika Westerberg Aug. 7, 2014, 8:43 a.m. UTC | #6

On Mon, Aug 04, 2014 at 08:55:50AM -0700, Darren Hart wrote:
> On 7/29/14, 15:06, "Andrea Adami" <[e-mail]> wrote:
> 
> >On Tue, Jul 29, 2014 at 8:55 PM, Mark Brown <[e-mail]> wrote:
> >> On Tue, Jul 29, 2014 at 06:13:06PM +0200, Andrea Adami wrote:
> >>
> >>> note that a previous version of the patch breaks spi on pxa machines:
> >>> tested on pxa250 poodle and pxa255 corgi.
> >>> The patch is included in linux-Yocto since 3.14.
> >>
> >> The interesting question is if this version works...
> >
> >Happy to report that I could finally test on poodle/pxa250 the current
> >v2 and the patch doesn't seem to disturb the probe.
> >Sorry for the noise.
> >
> >Andrea
> >
> 
> I tested this (v2) on the MinnowBoard-Max (Baytrail Atom E3825) in PCI
> mode and successfully attached an MCP32XX ADC and tested it's IIO
> interface.

Mark,

Any idea what happened to this patch? I didn't find it in linux-next and
it looks like it is not merged with mainline either.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kweh Hock Leong Aug. 13, 2014, 11:59 a.m. UTC | #7

> -----Original Message-----
> From: Mika Westerberg [mailto:[e-mail]]
> Sent: Thursday, August 07, 2014 4:43 PM
> To: Mark Brown; Darren Hart
> Cc: Andrea Adami; Chew, Chiau Ee; Kweh, Hock Leong; Russell King;
> [e-mail]; [e-mail]; Haojian Zhuang; linux-
> [e-mail]; Eric Miao; linux-arm-kernel
> Subject: Re: [PATCH v2] spi/pxa2xx-pci: Add common clock framework
> support in PCI glue layer
> 
> On Mon, Aug 04, 2014 at 08:55:50AM -0700, Darren Hart wrote:
> > On 7/29/14, 15:06, "Andrea Adami" <[e-mail]> wrote:
> >
> > >On Tue, Jul 29, 2014 at 8:55 PM, Mark Brown <[e-mail]> wrote:
> > >> On Tue, Jul 29, 2014 at 06:13:06PM +0200, Andrea Adami wrote:
> > >>
> > >>> note that a previous version of the patch breaks spi on pxa machines:
> > >>> tested on pxa250 poodle and pxa255 corgi.
> > >>> The patch is included in linux-Yocto since 3.14.
> > >>
> > >> The interesting question is if this version works...
> > >
> > >Happy to report that I could finally test on poodle/pxa250 the
> > >current
> > >v2 and the patch doesn't seem to disturb the probe.
> > >Sorry for the noise.
> > >
> > >Andrea
> > >
> >
> > I tested this (v2) on the MinnowBoard-Max (Baytrail Atom E3825) in PCI
> > mode and successfully attached an MCP32XX ADC and tested it's IIO
> > interface.
> 
> Mark,
> 
> Any idea what happened to this patch? I didn't find it in linux-next and it
> looks like it is not merged with mainline either.

Hi Mark,

Would like to follow up with you about this patch. Do you have plan to merge it in ?


Regards,
Wilson
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Brown Aug. 13, 2014, 7:20 p.m. UTC | #8

On Wed, Aug 13, 2014 at 11:59:14AM +0000, Kweh, Hock Leong wrote:

> > Any idea what happened to this patch? I didn't find it in linux-next and it
> > looks like it is not merged with mainline either.

> Would like to follow up with you about this patch. Do you have plan to merge it in ?
> 

Please don't send contentless pings, it just adds to the amount of mail
that needs reading.  Since I don't want to encourage people to do this I
tend to respond to contentless pings by delaying things, otherwise I
might give the impression that it's a good idea (and the "oh, I did
something with that" that comes from deleting the nag makes that happen
subconsciously anyway).

Please also allow a reasonable time for review, less than two weeks (to
when Mika first nagged) is a bit aggressive for something that isn't an
urgent bug fix.  People might take vacation or just be busy.

Mark Brown Aug. 16, 2014, 10:20 p.m. UTC | #9

On Fri, Jul 25, 2014 at 01:10:54AM +0800, [e-mail] wrote:
> From: Chew, Chiau Ee <[e-mail]>
> 
> SPI PXA2XX core layer has dependency on common clock framework
> to obtain information on host supported clock rate. Thus, we
> setup the clock device in the PCI glue layer to enable PCI mode
> host pass in the clock rate information.

Applied, thanks.

Patch

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 62e2242..aa005cb 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -380,7 +380,7 @@  config SPI_PXA2XX
 	  additional documentation can be found a Documentation/spi/pxa2xx.
 
 config SPI_PXA2XX_PCI
-	def_tristate SPI_PXA2XX && PCI
+	def_tristate SPI_PXA2XX && PCI && COMMON_CLK
 
 config SPI_ROCKCHIP
 	tristate "Rockchip SPI controller driver"
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index c1865c9..20ebbc7 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -7,6 +7,8 @@ 
 #include <linux/of_device.h>
 #include <linux/module.h>
 #include <linux/spi/pxa2xx_spi.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 
 enum {
 	PORT_CE4100,
@@ -21,6 +23,7 @@  struct pxa_spi_info {
 	int tx_chan_id;
 	int rx_slave_id;
 	int rx_chan_id;
+	unsigned long max_clk_rate;
 };
 
 static struct pxa_spi_info spi_info_configs[] = {
@@ -32,6 +35,7 @@  static struct pxa_spi_info spi_info_configs[] = {
 		.tx_chan_id = -1,
 		.rx_slave_id = -1,
 		.rx_chan_id = -1,
+		.max_clk_rate = 3686400,
 	},
 	[PORT_BYT] = {
 		.type = LPSS_SSP,
@@ -41,6 +45,7 @@  static struct pxa_spi_info spi_info_configs[] = {
 		.tx_chan_id = 0,
 		.rx_slave_id = 1,
 		.rx_chan_id = 1,
+		.max_clk_rate = 50000000,
 	},
 };
 
@@ -53,6 +58,7 @@  static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 	struct pxa2xx_spi_master spi_pdata;
 	struct ssp_device *ssp;
 	struct pxa_spi_info *c;
+	char buf[40];
 
 	ret = pcim_enable_device(dev);
 	if (ret)
@@ -84,6 +90,12 @@  static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 	ssp->port_id = (c->port_id >= 0) ? c->port_id : dev->devfn;
 	ssp->type = c->type;
 
+	snprintf(buf, sizeof(buf), "pxa2xx-spi.%d", ssp->port_id);
+	ssp->clk = clk_register_fixed_rate(&dev->dev, buf , NULL,
+					CLK_IS_ROOT, c->max_clk_rate);
+	 if (IS_ERR(ssp->clk))
+		return PTR_ERR(ssp->clk);
+
 	memset(&pi, 0, sizeof(pi));
 	pi.parent = &dev->dev;
 	pi.name = "pxa2xx-spi";
@@ -92,8 +104,10 @@  static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 	pi.size_data = sizeof(spi_pdata);
 
 	pdev = platform_device_register_full(&pi);
-	if (IS_ERR(pdev))
+	if (IS_ERR(pdev)) {
+		clk_unregister(ssp->clk);
 		return PTR_ERR(pdev);
+	}
 
 	pci_set_drvdata(dev, pdev);
 
@@ -103,8 +117,13 @@  static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 static void pxa2xx_spi_pci_remove(struct pci_dev *dev)
 {
 	struct platform_device *pdev = pci_get_drvdata(dev);
+	struct pxa2xx_spi_master *spi_pdata;
+
+	spi_pdata = dev_get_platdata(&pdev->dev);
 
 	platform_device_unregister(pdev);
+	clk_unregister(spi_pdata->ssp.clk);
+	pci_set_drvdata(dev, NULL);
 }
 
 static const struct pci_device_id pxa2xx_spi_pci_devices[] = {