diff mbox

[v2,2/2] spi: octeon: Add thunderx driver

Message ID 20160728083144.16625-3-jglauber@cavium.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Glauber July 28, 2016, 8:31 a.m. UTC
Add ThunderX SPI driver using the shared part from the Octeon
driver. The main difference of the ThunderX driver is that it
is a PCI device so probing is different. The system clock settings
can be specified in device tree.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/spi/Kconfig               |   7 ++
 drivers/spi/Makefile              |   2 +
 drivers/spi/spi-cavium-thunderx.c | 140 ++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-cavium.h          |   3 +
 4 files changed, 152 insertions(+)
 create mode 100644 drivers/spi/spi-cavium-thunderx.c

Comments

Mark Brown Aug. 1, 2016, 5:28 p.m. UTC | #1
On Thu, Jul 28, 2016 at 10:31:44AM +0200, Jan Glauber wrote:

> +config SPI_THUNDERX
> +	tristate "Cavium ThunderX SPI controller"
> +	depends on (ARM64 || CONFIG_TEST) && 64BIT && PCI

You mean COMPILE_TEST.

> +	p->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(p->clk))
> +		goto out_unmap;

We're now just using the normal clock API which is good but I'm now
unclear what is going to ensure that the clock is there - is there some
other change elsewhere that I'm not aware of?  We're also not passing
the resulting error code back to the caller which will break deferred
probe.

> +out_clock:
> +	clk_disable_unprepare(p->clk);
> +out_clock_devm:
> +	devm_clk_put(dev, p->clk);

There's no point in using managed allocations if you're going to manually
free things...
David Daney Aug. 1, 2016, 6:31 p.m. UTC | #2
On 08/01/2016 10:28 AM, Mark Brown wrote:
> On Thu, Jul 28, 2016 at 10:31:44AM +0200, Jan Glauber wrote:
>
>> +config SPI_THUNDERX
>> +	tristate "Cavium ThunderX SPI controller"
>> +	depends on (ARM64 || CONFIG_TEST) && 64BIT && PCI
>
> You mean COMPILE_TEST.

Yes, we will fix that typo.


>
>> +	p->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(p->clk))
>> +		goto out_unmap;
>
> We're now just using the normal clock API which is good but I'm now
> unclear what is going to ensure that the clock is there - is there some
> other change elsewhere that I'm not aware of?

The clock is an integral part of the SoC and is always running, so it 
will always be there.  All we want to know is the frequency, which is 
supplied by the device tree clock-bindings framework


> We're also not passing
> the resulting error code back to the caller which will break deferred
> probe.
>

Yes, we should do that.

>> +out_clock:
>> +	clk_disable_unprepare(p->clk);
>> +out_clock_devm:
>> +	devm_clk_put(dev, p->clk);
>
> There's no point in using managed allocations if you're going to manually
> free things...

Yes, we should let the automatic cleanup do its work here.

Probably we should consider using pcim_iomap(...) as well


>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Aug. 1, 2016, 6:49 p.m. UTC | #3
On Mon, Aug 01, 2016 at 11:31:43AM -0700, David Daney wrote:
> On 08/01/2016 10:28 AM, Mark Brown wrote:
> > On Thu, Jul 28, 2016 at 10:31:44AM +0200, Jan Glauber wrote:

> > > +	p->clk = devm_clk_get(dev, NULL);
> > > +	if (IS_ERR(p->clk))
> > > +		goto out_unmap;

> > We're now just using the normal clock API which is good but I'm now
> > unclear what is going to ensure that the clock is there - is there some
> > other change elsewhere that I'm not aware of?

> The clock is an integral part of the SoC and is always running, so it will
> always be there.  All we want to know is the frequency, which is supplied by
> the device tree clock-bindings framework

So there's something there that registers the clock?  What is that thing
on ACPI systems?
David Daney Aug. 1, 2016, 7:02 p.m. UTC | #4
On 08/01/2016 11:49 AM, Mark Brown wrote:
> On Mon, Aug 01, 2016 at 11:31:43AM -0700, David Daney wrote:
>> On 08/01/2016 10:28 AM, Mark Brown wrote:
>>> On Thu, Jul 28, 2016 at 10:31:44AM +0200, Jan Glauber wrote:
>
>>>> +	p->clk = devm_clk_get(dev, NULL);
>>>> +	if (IS_ERR(p->clk))
>>>> +		goto out_unmap;
>
>>> We're now just using the normal clock API which is good but I'm now
>>> unclear what is going to ensure that the clock is there - is there some
>>> other change elsewhere that I'm not aware of?
>
>> The clock is an integral part of the SoC and is always running, so it will
>> always be there.  All we want to know is the frequency, which is supplied by
>> the device tree clock-bindings framework
>
> So there's something there that registers the clock?

Yes, when using OF device tree, standard device probing registers the clock.

> What is that thing on ACPI systems?

I don't know if it works ACPI, or if ACPI even has support for the clock 
framework.  But does it matter?  We are not currently using ACPI on 
systems where this driver is used.

In the future, if we ever need ACPI support, we will add support for it.

David.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Aug. 2, 2016, 9:30 p.m. UTC | #5
On Mon, Aug 01, 2016 at 12:02:54PM -0700, David Daney wrote:
> On 08/01/2016 11:49 AM, Mark Brown wrote:

> > What is that thing on ACPI systems?

> I don't know if it works ACPI, or if ACPI even has support for the clock
> framework.  But does it matter?  We are not currently using ACPI on systems
> where this driver is used.

> In the future, if we ever need ACPI support, we will add support for it.

Oh, that's surprising - I thought these were server systems.  With ACPI
you need to use DMI data or something to instantiate the clock.  In any
case if you've got it working that should be OK.
David Daney Aug. 2, 2016, 9:49 p.m. UTC | #6
On 08/02/2016 02:30 PM, Mark Brown wrote:
> On Mon, Aug 01, 2016 at 12:02:54PM -0700, David Daney wrote:
>> On 08/01/2016 11:49 AM, Mark Brown wrote:
>
>>> What is that thing on ACPI systems?
>
>> I don't know if it works ACPI, or if ACPI even has support for the clock
>> framework.  But does it matter?  We are not currently using ACPI on systems
>> where this driver is used.
>
>> In the future, if we ever need ACPI support, we will add support for it.
>
> Oh, that's surprising - I thought these were server systems.

There are two broad classes of systems targeted by Cavium's arm64 based 
SoCs:

1) 2-node NUMA servers with 96 CPUs running UEFI firmware and ACPI.  For 
these, any SPI buses are managed by the firmware and we don't need 
Kernel support.

2) Embedded controllers with 4 - 32 CPUs, typically running u-boot and 
OF device tree firmware descriptions.  For these configurations, SPI 
buses managed by the kernel must be supported.


> With ACPI
> you need to use DMI data or something to instantiate the clock.  In any
> case if you've got it working that should be OK.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 4b931ec..e0ee112 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -630,6 +630,13 @@  config SPI_TEGRA20_SLINK
 	help
 	  SPI driver for Nvidia Tegra20/Tegra30 SLINK Controller interface.
 
+config SPI_THUNDERX
+	tristate "Cavium ThunderX SPI controller"
+	depends on (ARM64 || CONFIG_TEST) && 64BIT && PCI
+	help
+	  SPI host driver for the hardware found on Cavium ThunderX
+	  SOCs.
+
 config SPI_TOPCLIFF_PCH
 	tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) SPI"
 	depends on PCI && (X86_32 || MIPS || COMPILE_TEST)
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 185367e..133364b 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -91,6 +91,8 @@  obj-$(CONFIG_SPI_TEGRA114)		+= spi-tegra114.o
 obj-$(CONFIG_SPI_TEGRA20_SFLASH)	+= spi-tegra20-sflash.o
 obj-$(CONFIG_SPI_TEGRA20_SLINK)		+= spi-tegra20-slink.o
 obj-$(CONFIG_SPI_TLE62X0)		+= spi-tle62x0.o
+spi-thunderx-objs			:= spi-cavium.o spi-cavium-thunderx.o
+obj-$(CONFIG_SPI_THUNDERX)		+= spi-thunderx.o
 obj-$(CONFIG_SPI_TOPCLIFF_PCH)		+= spi-topcliff-pch.o
 obj-$(CONFIG_SPI_TXX9)			+= spi-txx9.o
 obj-$(CONFIG_SPI_XCOMM)		+= spi-xcomm.o
diff --git a/drivers/spi/spi-cavium-thunderx.c b/drivers/spi/spi-cavium-thunderx.c
new file mode 100644
index 0000000..28c3dcc
--- /dev/null
+++ b/drivers/spi/spi-cavium-thunderx.c
@@ -0,0 +1,140 @@ 
+/*
+ * Cavium ThunderX SPI driver.
+ *
+ * Copyright (C) 2016 Cavium Inc.
+ * Authors: Jan Glauber <jglauber@cavium.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/spi/spi.h>
+
+#include "spi-cavium.h"
+
+#define DRV_NAME "spi-thunderx"
+
+#define SYS_FREQ_DEFAULT 700000000 /* 700 Mhz */
+
+static int thunderx_spi_probe(struct pci_dev *pdev,
+			      const struct pci_device_id *ent)
+{
+	struct device *dev = &pdev->dev;
+	struct spi_master *master;
+	struct octeon_spi *p;
+	int ret = -ENOENT;
+
+	master = spi_alloc_master(dev, sizeof(struct octeon_spi));
+	if (!master)
+		return -ENOMEM;
+	p = spi_master_get_devdata(master);
+
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		dev_err(dev, "Failed to enable PCI device\n");
+		goto out_free;
+	}
+
+	ret = pci_request_regions(pdev, DRV_NAME);
+	if (ret) {
+		dev_err(dev, "PCI request regions failed 0x%x\n", ret);
+		goto out_disable;
+	}
+
+	p->register_base = pci_ioremap_bar(pdev, 0);
+	if (!p->register_base) {
+		dev_err(dev, "Cannot map reg base\n");
+		ret = -EINVAL;
+		goto out_region;
+	}
+
+	p->regs.config = 0x1000;
+	p->regs.status = 0x1008;
+	p->regs.tx = 0x1010;
+	p->regs.data = 0x1080;
+
+	p->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(p->clk))
+		goto out_unmap;
+
+	ret = clk_prepare_enable(p->clk);
+	if (ret)
+		goto out_clock_devm;
+
+	p->sys_freq = clk_get_rate(p->clk);
+	if (!p->sys_freq)
+		p->sys_freq = SYS_FREQ_DEFAULT;
+	dev_info(dev, "Set system clock to %u\n", p->sys_freq);
+
+	master->num_chipselect = 4;
+	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_CS_HIGH |
+			    SPI_LSB_FIRST | SPI_3WIRE;
+	master->transfer_one_message = octeon_spi_transfer_one_message;
+	master->bits_per_word_mask = SPI_BPW_MASK(8);
+	master->max_speed_hz = OCTEON_SPI_MAX_CLOCK_HZ;
+	master->dev.of_node = pdev->dev.of_node;
+
+	pci_set_drvdata(pdev, master);
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(&pdev->dev, "Register master failed: %d\n", ret);
+		goto out_clock;
+	}
+
+	return 0;
+
+out_clock:
+	clk_disable_unprepare(p->clk);
+out_clock_devm:
+	devm_clk_put(dev, p->clk);
+out_unmap:
+	iounmap(p->register_base);
+out_region:
+	pci_release_regions(pdev);
+out_disable:
+	pci_disable_device(pdev);
+out_free:
+	spi_master_put(master);
+	return ret;
+}
+
+static void thunderx_spi_remove(struct pci_dev *pdev)
+{
+	struct spi_master *master = pci_get_drvdata(pdev);
+	struct octeon_spi *p;
+
+	p = spi_master_get_devdata(master);
+	if (!p)
+		return;
+
+	/* Put everything in a known state. */
+	writeq(0, p->register_base + OCTEON_SPI_CFG(p));
+
+	clk_disable_unprepare(p->clk);
+	devm_clk_put(&pdev->dev, p->clk);
+	iounmap(p->register_base);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+static const struct pci_device_id thunderx_spi_pci_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa00b) },
+	{ 0, }
+};
+
+MODULE_DEVICE_TABLE(pci, thunderx_spi_pci_id_table);
+
+static struct pci_driver thunderx_spi_driver = {
+	.name		= DRV_NAME,
+	.id_table	= thunderx_spi_pci_id_table,
+	.probe		= thunderx_spi_probe,
+	.remove		= thunderx_spi_remove,
+};
+
+module_pci_driver(thunderx_spi_driver);
+
+MODULE_DESCRIPTION("Cavium, Inc. ThunderX SPI bus driver");
+MODULE_AUTHOR("Jan Glauber");
+MODULE_LICENSE("GPL");
diff --git a/drivers/spi/spi-cavium.h b/drivers/spi/spi-cavium.h
index 88c5f36..1f91d61 100644
--- a/drivers/spi/spi-cavium.h
+++ b/drivers/spi/spi-cavium.h
@@ -1,6 +1,8 @@ 
 #ifndef __SPI_CAVIUM_H
 #define __SPI_CAVIUM_H
 
+#include <linux/clk.h>
+
 #define OCTEON_SPI_MAX_BYTES 9
 #define OCTEON_SPI_MAX_CLOCK_HZ 16000000
 
@@ -17,6 +19,7 @@  struct octeon_spi {
 	u64 cs_enax;
 	int sys_freq;
 	struct octeon_spi_regs regs;
+	struct clk *clk;
 };
 
 #define OCTEON_SPI_CFG(x)	(x->regs.config)