Message ID | 20160728083144.16625-3-jglauber@cavium.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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...
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
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?
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
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.
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 --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)
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