[PATCH v4 0/2] J-Core SPI controller support

[PATCH v4 0/2] J-Core SPI controller support

[Rich Felker]

This driver is used with the J2, an open-source VHDL reimplementation
of SH-2. I've split it out from the main J-Core support patches going
upstream through my own linux-sh tree. Changes since the v3 should
include everything requested by Mark Brown in previous review (mainly
the chipsel changes I failed to make before), plus significant cleanup
(removal of unused macros and other cruft) and refactoring to
eliminate duplicate code.

Rich


Rich Felker (2):
  of: add J-Core SPI master bindings
  spi: add driver for J-Core SPI controller

 .../devicetree/bindings/spi/jcore,spi.txt          |  30 +++
 drivers/spi/Kconfig                                |   4 +
 drivers/spi/Makefile                               |   1 +
 drivers/spi/spi-jcore.c                            | 220 +++++++++++++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt
 create mode 100644 drivers/spi/spi-jcore.c

-- 
2.8.1

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

[PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Rich Felker]

The J-Core "spi2" device is a PIO-based SPI master controller. It
differs from "bitbang" devices in that that it's clocked in hardware
rather than via soft clock modulation over gpio, and performs
byte-at-a-time transfers between the cpu and SPI controller.

This driver will be extended to support future versions of the J-Core
SPI controller with DMA transfers when they become available.

Signed-off-by: Rich Felker <dalias@libc.org>
---
 drivers/spi/Kconfig     |   4 +
 drivers/spi/Makefile    |   1 +
 drivers/spi/spi-jcore.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 225 insertions(+)
 create mode 100644 drivers/spi/spi-jcore.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 4b931ec..630ad7c 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -285,6 +285,10 @@ config SPI_IMX
 	  This enables using the Freescale i.MX SPI controllers in master
 	  mode.
 
+config SPI_JCORE
+	tristate "J-Core SPI Master"
+	depends on OF
+
 config SPI_LM70_LLP
 	tristate "Parallel port adapter for LM70 eval board (DEVELOPMENT)"
 	depends on PARPORT
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 3c74d00..7e11fef 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_SPI_FSL_SPI)		+= spi-fsl-spi.o
 obj-$(CONFIG_SPI_GPIO)			+= spi-gpio.o
 obj-$(CONFIG_SPI_IMG_SPFI)		+= spi-img-spfi.o
 obj-$(CONFIG_SPI_IMX)			+= spi-imx.o
+obj-$(CONFIG_SPI_JCORE)			+= spi-jcore.o
 obj-$(CONFIG_SPI_LM70_LLP)		+= spi-lm70llp.o
 obj-$(CONFIG_SPI_LP8841_RTC)		+= spi-lp8841-rtc.o
 obj-$(CONFIG_SPI_MESON_SPIFC)		+= spi-meson-spifc.o
diff --git a/drivers/spi/spi-jcore.c b/drivers/spi/spi-jcore.c
new file mode 100644
index 0000000..7d328e0
--- /dev/null
+++ b/drivers/spi/spi-jcore.c
@@ -0,0 +1,220 @@
+/*
+ * J-Core SPI controller driver
+ *
+ * Copyright (C) 2012-2016 Smart Energy Instruments, Inc.
+ *
+ * Current version by Rich Felker
+ * Based loosely on initial version by Oleksandr G Zhadan
+ *
+ */
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/delay.h>
+
+#define DRV_NAME "jcore_spi"
+
+#define CTRL_REG	0x0
+#define DATA_REG	0x4
+
+#define JCORE_SPI_CTRL_XMIT		0x02
+#define JCORE_SPI_STAT_BUSY		0x02
+#define JCORE_SPI_CTRL_LOOP		0x08
+#define JCORE_SPI_CTRL_CS_BITS		0x15
+
+#define JCORE_SPI_WAIT_RDY_MAX_LOOP	2000000
+
+struct jcore_spi {
+	struct spi_master *master;
+	void __iomem *base;
+	unsigned int cs_reg;
+	unsigned int speed_reg;
+	unsigned int speed_hz;
+	unsigned int clock_freq;
+};
+
+static int jcore_spi_wait(void __iomem *ctrl_reg)
+{
+	unsigned timeout = JCORE_SPI_WAIT_RDY_MAX_LOOP;
+
+	do {
+		if (!(readl(ctrl_reg) & JCORE_SPI_STAT_BUSY))
+			return 0;
+		cpu_relax();
+	} while (--timeout);
+
+	return -EBUSY;
+}
+
+static void jcore_spi_program(struct jcore_spi *hw)
+{
+	void __iomem *ctrl_reg = hw->base + CTRL_REG;
+
+	if (jcore_spi_wait(ctrl_reg))
+		dev_err(hw->master->dev.parent,
+			"timeout waiting to program ctrl reg.\n");
+
+	writel(hw->cs_reg | hw->speed_reg, ctrl_reg);
+}
+
+static void jcore_spi_chipsel(struct spi_device *spi, bool value)
+{
+	struct jcore_spi *hw = spi_master_get_devdata(spi->master);
+	u32 csbit = 1U << (2 * spi->chip_select);
+
+	dev_dbg(hw->master->dev.parent, "chipselect %d\n", spi->chip_select);
+
+	if (value)
+		hw->cs_reg |= csbit;
+	else
+		hw->cs_reg &= ~csbit;
+
+	jcore_spi_program(hw);
+}
+
+static void jcore_spi_baudrate(struct jcore_spi *hw, int speed)
+{
+	if (speed == hw->speed_hz) return;
+	hw->speed_hz = speed;
+	if (speed >= hw->clock_freq/2)
+		hw->speed_reg = 0;
+	else
+		hw->speed_reg = ((hw->clock_freq / 2 / speed) - 1) << 27;
+	jcore_spi_program(hw);
+	dev_dbg(hw->master->dev.parent, "speed=%d reg=0x%x\n",
+		speed, hw->speed_reg);
+}
+
+static int jcore_spi_txrx(struct spi_master *master, struct spi_device *spi,
+			  struct spi_transfer *t)
+{
+	struct jcore_spi *hw = spi_master_get_devdata(master);
+
+	void __iomem *ctrl_reg = hw->base + CTRL_REG;
+	void __iomem *data_reg = hw->base + DATA_REG;
+	u32 xmit;
+
+	/* data buffers */
+	const unsigned char *tx;
+	unsigned char *rx;
+	unsigned int len;
+	unsigned int count;
+
+	jcore_spi_baudrate(hw, t->speed_hz);
+
+	xmit = hw->cs_reg | hw->speed_reg | JCORE_SPI_CTRL_XMIT;
+	tx = t->tx_buf;
+	rx = t->rx_buf;
+	len = t->len;
+
+	for (count = 0; count < len; count++) {
+		if (jcore_spi_wait(ctrl_reg))
+			break;
+
+		writel(tx ? *tx++ : 0, data_reg);
+		writel(xmit, ctrl_reg);
+
+		if (jcore_spi_wait(ctrl_reg))
+			break;
+
+		if (rx)
+			*rx++ = readl(data_reg);
+	}
+
+	spi_finalize_current_transfer(master);
+
+	return count<len ? -EREMOTEIO : 0;
+}
+
+static int jcore_spi_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct jcore_spi *hw;
+	struct spi_master *master;
+	struct resource *res;
+	u32 clock_freq;
+	int err = -ENODEV;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(struct jcore_spi));
+	if (!master)
+		return err;
+
+	/* setup the master state. */
+	master->num_chipselect = 3;
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	master->transfer_one = jcore_spi_txrx;
+	master->set_cs = jcore_spi_chipsel;
+	master->dev.of_node = node;
+	master->bus_num = pdev->id;
+
+	hw = spi_master_get_devdata(master);
+	hw->master = master;
+	platform_set_drvdata(pdev, hw);
+
+	/* find and map our resources */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		goto exit_busy;
+	if (!devm_request_mem_region
+	    (&pdev->dev, res->start, resource_size(res), pdev->name))
+		goto exit_busy;
+	hw->base =
+	    devm_ioremap_nocache(&pdev->dev, res->start, resource_size(res));
+	if (!hw->base)
+		goto exit_busy;
+
+	/* The SPI clock rate controlled via a configurable clock divider
+	 * which is applied to the reference clock. A 50 MHz reference is
+	 * most suitable for obtaining standard SPI clock rates, but some
+	 * designs may have a different reference clock, and the DT must
+	 * make the driver aware so that it can properly program the
+	 * requested rate. If omitted, 50 MHz is assumed. */
+	clock_freq = 50000000;
+	of_property_read_u32(node, "clock-frequency", &clock_freq);
+	hw->clock_freq = clock_freq;
+
+	/* Initialize all CS bits to high. */
+	hw->cs_reg = JCORE_SPI_CTRL_CS_BITS;
+	jcore_spi_baudrate(hw, 400000);
+
+	pdev->dev.dma_mask = 0;
+	/* register our spi controller */
+	err = devm_spi_register_master(&pdev->dev, master);
+	if (err)
+		goto exit;
+	dev_info(&pdev->dev, "base %p, noirq\n", hw->base);
+
+	return 0;
+
+exit_busy:
+	err = -EBUSY;
+exit:
+	platform_set_drvdata(pdev, NULL);
+	spi_master_put(master);
+	return err;
+}
+
+static const struct of_device_id jcore_spi_of_match[] = {
+	{ .compatible = "jcore,spi2" },
+	{},
+};
+
+static struct platform_driver jcore_spi_driver = {
+	.probe = jcore_spi_probe,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = jcore_spi_of_match,
+	},
+};
+
+module_platform_driver(jcore_spi_driver);
+
+MODULE_DESCRIPTION("J-Core SPI driver");
+MODULE_AUTHOR("Rich Felker <dalias@libc.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.8.1

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]

On Sun, Apr 03, 2016 at 05:12:45AM +0000, Rich Felker wrote:

> +config SPI_JCORE
> +	tristate "J-Core SPI Master"
> +	depends on OF
> +

An architecture or SoC dependency with || COMPILE_TEST would be useful
for avoiding cluttering Kconfig for other users.  Though as this is in a
FPGA it's perhaps likely people will pick this up for other FPGAs so
perhaps a comment to that effect if it seems likely.

> +static void jcore_spi_baudrate(struct jcore_spi *hw, int speed)
> +{
> +	if (speed == hw->speed_hz) return;
> +	hw->speed_hz = speed;
> +	if (speed >= hw->clock_freq/2)

Coding style, spaces around /.

> +	return count<len ? -EREMOTEIO : 0;

Again here, and please also write this as a normal if statement so it's
easier to read.

> +	/* The SPI clock rate controlled via a configurable clock divider
> +	 * which is applied to the reference clock. A 50 MHz reference is
> +	 * most suitable for obtaining standard SPI clock rates, but some
> +	 * designs may have a different reference clock, and the DT must
> +	 * make the driver aware so that it can properly program the
> +	 * requested rate. If omitted, 50 MHz is assumed. */
> +	clock_freq = 50000000;
> +	of_property_read_u32(node, "clock-frequency", &clock_freq);
> +	hw->clock_freq = clock_freq;

Why are you not using the clock API for this?  Just require a clock and
use clk_get_rate() to find out what rate it is.

> +	pdev->dev.dma_mask = 0;

Why are we doing this?  There's no DMA code...

> +	dev_info(&pdev->dev, "base %p, noirq\n", hw->base);

This is just adding noise to the boot, just remove it - it's useful to
log information we get from the silicon but things like the base address
don't really add anything and end up cluttering (and slowing) the boot
when everything does it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Rich Felker]

On Thu, Jul 28, 2016 at 08:11:53PM +0100, Mark Brown wrote:
> On Sun, Apr 03, 2016 at 05:12:45AM +0000, Rich Felker wrote:
> 
> > +config SPI_JCORE
> > +	tristate "J-Core SPI Master"
> > +	depends on OF
> > +
> 
> An architecture or SoC dependency with || COMPILE_TEST would be useful
> for avoiding cluttering Kconfig for other users.  Though as this is in a
> FPGA it's perhaps likely people will pick this up for other FPGAs so
> perhaps a comment to that effect if it seems likely.

Unlike some of the other SoC hardware (interrupt controller) that's
more closely tied to the SH cpu trap behavior, the SPI master seems
like something that would be nice and easy to reuse elsewhere. I don't
feel strongly about it either way though; I can add the arch dep if
you want.

> > +static void jcore_spi_baudrate(struct jcore_spi *hw, int speed)
> > +{
> > +	if (speed == hw->speed_hz) return;
> > +	hw->speed_hz = speed;
> > +	if (speed >= hw->clock_freq/2)
> 
> Coding style, spaces around /.

OK.

> > +	return count<len ? -EREMOTEIO : 0;
> 
> Again here, and please also write this as a normal if statement so it's
> easier to read.

OK.

> > +	/* The SPI clock rate controlled via a configurable clock divider
> > +	 * which is applied to the reference clock. A 50 MHz reference is
> > +	 * most suitable for obtaining standard SPI clock rates, but some
> > +	 * designs may have a different reference clock, and the DT must
> > +	 * make the driver aware so that it can properly program the
> > +	 * requested rate. If omitted, 50 MHz is assumed. */
> > +	clock_freq = 50000000;
> > +	of_property_read_u32(node, "clock-frequency", &clock_freq);
> > +	hw->clock_freq = clock_freq;
> 
> Why are you not using the clock API for this?  Just require a clock and
> use clk_get_rate() to find out what rate it is.

I thought about that but I'm not familiar with it. I can try to figure
it out quickly and test that approach; don't see any reason it
shouldn't work. Would you insist on having full support for
enabling/disabling the clk when it's in use, or would you be happy
with treating it as a fixed clock that's always-on for now and
possibly extending it with more functionality later if there's ever
hardware where that's relevant/helpful?

> > +	pdev->dev.dma_mask = 0;
> 
> Why are we doing this?  There's no DMA code...

There will be later, but I can remove this for now if it's not needed.

> > +	dev_info(&pdev->dev, "base %p, noirq\n", hw->base);
> 
> This is just adding noise to the boot, just remove it - it's useful to
> log information we get from the silicon but things like the base address
> don't really add anything and end up cluttering (and slowing) the boot
> when everything does it.

OK.

Rich

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

On Thu, Jul 28, 2016 at 03:40:45PM -0400, Rich Felker wrote:
> On Thu, Jul 28, 2016 at 08:11:53PM +0100, Mark Brown wrote:

> > An architecture or SoC dependency with || COMPILE_TEST would be useful
> > for avoiding cluttering Kconfig for other users.  Though as this is in a
> > FPGA it's perhaps likely people will pick this up for other FPGAs so
> > perhaps a comment to that effect if it seems likely.

> Unlike some of the other SoC hardware (interrupt controller) that's
> more closely tied to the SH cpu trap behavior, the SPI master seems
> like something that would be nice and easy to reuse elsewhere. I don't
> feel strongly about it either way though; I can add the arch dep if
> you want.

I guess it depends if anyone is actually doing that or not, if nobody is
the dependency would be better.

> > Why are you not using the clock API for this?  Just require a clock and
> > use clk_get_rate() to find out what rate it is.

> I thought about that but I'm not familiar with it. I can try to figure
> it out quickly and test that approach; don't see any reason it
> shouldn't work. Would you insist on having full support for
> enabling/disabling the clk when it's in use, or would you be happy
> with treating it as a fixed clock that's always-on for now and
> possibly extending it with more functionality later if there's ever
> hardware where that's relevant/helpful?

It's fine to just enable it at startup and leave it on, though the
runtime PM ops are trivial and you can set auto_runtime_pm to have the
core do the gets and puts.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Rich Felker]

On Thu, Jul 28, 2016 at 08:51:25PM +0100, Mark Brown wrote:
> On Thu, Jul 28, 2016 at 03:40:45PM -0400, Rich Felker wrote:
> > On Thu, Jul 28, 2016 at 08:11:53PM +0100, Mark Brown wrote:
> 
> > > An architecture or SoC dependency with || COMPILE_TEST would be useful
> > > for avoiding cluttering Kconfig for other users.  Though as this is in a
> > > FPGA it's perhaps likely people will pick this up for other FPGAs so
> > > perhaps a comment to that effect if it seems likely.
> 
> > Unlike some of the other SoC hardware (interrupt controller) that's
> > more closely tied to the SH cpu trap behavior, the SPI master seems
> > like something that would be nice and easy to reuse elsewhere. I don't
> > feel strongly about it either way though; I can add the arch dep if
> > you want.
> 
> I guess it depends if anyone is actually doing that or not, if nobody is
> the dependency would be better.

OK, let's add the dependency for now with the intent to remove it
if/when there's a need.

> > > Why are you not using the clock API for this?  Just require a clock and
> > > use clk_get_rate() to find out what rate it is.
> 
> > I thought about that but I'm not familiar with it. I can try to figure
> > it out quickly and test that approach; don't see any reason it
> > shouldn't work. Would you insist on having full support for
> > enabling/disabling the clk when it's in use, or would you be happy
> > with treating it as a fixed clock that's always-on for now and
> > possibly extending it with more functionality later if there's ever
> > hardware where that's relevant/helpful?
> 
> It's fine to just enable it at startup and leave it on, though the
> runtime PM ops are trivial and you can set auto_runtime_pm to have the
> core do the gets and puts.

I was able to get it working via the clk api and I'll include support
for this in the next version of the patch, but to actually use it
depends on changing arch/sh to use the common clk framework; otherwise
there's no way to provide a suitable clk in the DT and have
[devm_]clk_get actually pick it up. Should I keep around the option of
using clock-frequency too? That would be most convenient.

I do have a pending patch from Sato-san to switch arch/sh over to CCF
but it's part of a series and I don't think it's ready to merge. I may
be able to merge just a minimal, safe subset that won't break legacy
non-DT configurations, though.

Rich

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

On Fri, Jul 29, 2016 at 11:34:41PM -0400, Rich Felker wrote:

> I was able to get it working via the clk api and I'll include support
> for this in the next version of the patch, but to actually use it
> depends on changing arch/sh to use the common clk framework; otherwise
> there's no way to provide a suitable clk in the DT and have
> [devm_]clk_get actually pick it up. Should I keep around the option of
> using clock-frequency too? That would be most convenient.

It sounds like at the minute the devices all have one frequency anyway
in which case just having a default frequency in there that you fall
back to if you get -ENOENT from a failed clock lookup might be enough?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Rich Felker]

On Mon, Aug 01, 2016 at 07:12:45PM +0100, Mark Brown wrote:
> On Fri, Jul 29, 2016 at 11:34:41PM -0400, Rich Felker wrote:
> 
> > I was able to get it working via the clk api and I'll include support
> > for this in the next version of the patch, but to actually use it
> > depends on changing arch/sh to use the common clk framework; otherwise
> > there's no way to provide a suitable clk in the DT and have
> > [devm_]clk_get actually pick it up. Should I keep around the option of
> > using clock-frequency too? That would be most convenient.
> 
> It sounds like at the minute the devices all have one frequency anyway
> in which case just having a default frequency in there that you fall
> back to if you get -ENOENT from a failed clock lookup might be enough?

Yes. 50 MHz is the natural default frequency, but I found out at the
last minute from the hardware engineers that clocking the current SoC
up to 62.5 MHz (for faster cpu) will require the SPI timing to be
programmed based on the faster reference clock. This messes up the
ability to get optimal SD card transfer rates, so we'll probably end
up having a real 50 MHz clock for the SPI anyway, but I thought it was
important to be able to handle this issue in the DT binding anyway.

Since it's not an immediate need right now anyway I'll drop the
clock-frequency property proposal and just use the more general clocks
approach you suggested.

Rich
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Rob Landley]

On 08/01/2016 08:45 PM, Rich Felker wrote:
> Yes. 50 MHz is the natural default frequency, but I found out at the
> last minute from the hardware engineers that clocking the current SoC
> up to 62.5 MHz (for faster cpu) will require the SPI timing to be
> programmed based on the faster reference clock. This messes up the
> ability to get optimal SD card transfer rates, so we'll probably end
> up having a real 50 MHz clock for the SPI anyway, but I thought it was
> important to be able to handle this issue in the DT binding anyway.

For those of you following along at home, the first open source VHDL
release of http://j-core.org runs at 33 mhz on Spartan 6 FPGAs, the
second at 50mhz on the same hardware, and you'd think the third would be
66mhz as the chip continues to be optimized, but one of the new I/O
busses requires a multiple of 12.5 mhz and they didn't want to add a
clock domain crossing thing for it, hence 62.5 mhz.

Earlier it was easy to get everything to work off a single master clock,
but as we add more peripherals to the SOC and speed things up stuff's
likely to change. (Development is ongoing, and we have a public
http://j-core.org/roadmap.html out through 2019. It you're waiting for
stuff to stop changing before getting basic board support in that runs
on existing releases, it's might be a while.)

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

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Geert Uytterhoeven]

Hi Rich,

On Sat, Jul 30, 2016 at 5:34 AM, Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org> wrote:
> On Thu, Jul 28, 2016 at 08:51:25PM +0100, Mark Brown wrote:
>> On Thu, Jul 28, 2016 at 03:40:45PM -0400, Rich Felker wrote:
>> > On Thu, Jul 28, 2016 at 08:11:53PM +0100, Mark Brown wrote:
>> > > Why are you not using the clock API for this?  Just require a clock and
>> > > use clk_get_rate() to find out what rate it is.
>>
>> > I thought about that but I'm not familiar with it. I can try to figure
>> > it out quickly and test that approach; don't see any reason it
>> > shouldn't work. Would you insist on having full support for
>> > enabling/disabling the clk when it's in use, or would you be happy
>> > with treating it as a fixed clock that's always-on for now and
>> > possibly extending it with more functionality later if there's ever
>> > hardware where that's relevant/helpful?
>>
>> It's fine to just enable it at startup and leave it on, though the
>> runtime PM ops are trivial and you can set auto_runtime_pm to have the
>> core do the gets and puts.
>
> I was able to get it working via the clk api and I'll include support
> for this in the next version of the patch, but to actually use it
> depends on changing arch/sh to use the common clk framework; otherwise
> there's no way to provide a suitable clk in the DT and have
> [devm_]clk_get actually pick it up. Should I keep around the option of
> using clock-frequency too? That would be most convenient.
>
> I do have a pending patch from Sato-san to switch arch/sh over to CCF
> but it's part of a series and I don't think it's ready to merge. I may
> be able to merge just a minimal, safe subset that won't break legacy
> non-DT configurations, though.

I think you can use non-CCF clocks with DT, if you register them first.
Cfr. the clk_names[] array and shmobile_clk_workaround() function in
v3.18:arch/arm/mach-shmobile/board-koelsch-reference.c and
v3.18:arch/arm/mach-shmobile/clock.c

Or was that the other way around?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] spi: add driver for J-Core SPI controller

[Rich Felker]

On Thu, Aug 04, 2016 at 03:05:16PM +0200, Geert Uytterhoeven wrote:
> Hi Rich,
> 
> On Sat, Jul 30, 2016 at 5:34 AM, Rich Felker <dalias@libc.org> wrote:
> > On Thu, Jul 28, 2016 at 08:51:25PM +0100, Mark Brown wrote:
> >> On Thu, Jul 28, 2016 at 03:40:45PM -0400, Rich Felker wrote:
> >> > On Thu, Jul 28, 2016 at 08:11:53PM +0100, Mark Brown wrote:
> >> > > Why are you not using the clock API for this?  Just require a clock and
> >> > > use clk_get_rate() to find out what rate it is.
> >>
> >> > I thought about that but I'm not familiar with it. I can try to figure
> >> > it out quickly and test that approach; don't see any reason it
> >> > shouldn't work. Would you insist on having full support for
> >> > enabling/disabling the clk when it's in use, or would you be happy
> >> > with treating it as a fixed clock that's always-on for now and
> >> > possibly extending it with more functionality later if there's ever
> >> > hardware where that's relevant/helpful?
> >>
> >> It's fine to just enable it at startup and leave it on, though the
> >> runtime PM ops are trivial and you can set auto_runtime_pm to have the
> >> core do the gets and puts.
> >
> > I was able to get it working via the clk api and I'll include support
> > for this in the next version of the patch, but to actually use it
> > depends on changing arch/sh to use the common clk framework; otherwise
> > there's no way to provide a suitable clk in the DT and have
> > [devm_]clk_get actually pick it up. Should I keep around the option of
> > using clock-frequency too? That would be most convenient.
> >
> > I do have a pending patch from Sato-san to switch arch/sh over to CCF
> > but it's part of a series and I don't think it's ready to merge. I may
> > be able to merge just a minimal, safe subset that won't break legacy
> > non-DT configurations, though.
> 
> I think you can use non-CCF clocks with DT, if you register them first.
> Cfr. the clk_names[] array and shmobile_clk_workaround() function in
> v3.18:arch/arm/mach-shmobile/board-koelsch-reference.c and
> v3.18:arch/arm/mach-shmobile/clock.c
> 
> Or was that the other way around?

You can, but they will only be registered if a board file or
equivalent registers them. The intent is not to have any board files
for boards supported through device tree and eventually to remove all
the board files from arch/sh.

Rich

[PATCH v4 1/2] of: add J-Core SPI master bindings

[Rich Felker]

Signed-off-by: Rich Felker <dalias@libc.org>
---
 .../devicetree/bindings/spi/jcore,spi.txt          | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt

diff --git a/Documentation/devicetree/bindings/spi/jcore,spi.txt b/Documentation/devicetree/bindings/spi/jcore,spi.txt
new file mode 100644
index 0000000..815106d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/jcore,spi.txt
@@ -0,0 +1,30 @@
+J-Core SPI master
+
+Required properties:
+
+- compatible: Must be "jcore,spi2".
+
+- reg: Memory region for registers.
+
+- #address-cells: Must be 1.
+
+- #size-cells: Must be 0.
+
+Optional properties:
+
+- clock-frequency: Specifies an input clock frequency relative to
+  which the SPI clock speed must be programmed. Necessary only if
+  the input clock rate is something other than 50 MHz.
+
+See spi-bus.txt for additional properties not specific to this device.
+
+Example:
+
+spi@40 {
+	compatible = "jcore,spi2";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x40 0x8>;
+	spi-max-frequency = <25000000>;
+	clock-frequency = <50000000>;
+}
-- 
2.8.1

Re: [PATCH v4 1/2] of: add J-Core SPI master bindings

[Rob Herring]

On Tue, May 17, 2016 at 11:19:25PM +0000, Rich Felker wrote:
> Signed-off-by: Rich Felker <dalias@libc.org>
> ---
>  .../devicetree/bindings/spi/jcore,spi.txt          | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 1/2] of: add J-Core SPI master bindings

[Rich Felker]

On Fri, Jul 29, 2016 at 03:58:38PM -0500, Rob Herring wrote:
> On Tue, May 17, 2016 at 11:19:25PM +0000, Rich Felker wrote:
> > Signed-off-by: Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org>
> > ---
> >  .../devicetree/bindings/spi/jcore,spi.txt          | 30 ++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Thanks. Since I switched to to using the clk framework rather than a
clock-frequency property (at Mark Brown's suggestion/request), I'll be
submitting one more version for your review.

After that, who should be responsible for upstreaming this and the
other binding patches? You, me, or the subsystem maintainers that the
drivers are going through?

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

Re: [PATCH v4 1/2] of: add J-Core SPI master bindings

[Rob Herring]

On Tue, Aug 2, 2016 at 5:00 PM, Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org> wrote:
> On Fri, Jul 29, 2016 at 03:58:38PM -0500, Rob Herring wrote:
>> On Tue, May 17, 2016 at 11:19:25PM +0000, Rich Felker wrote:
>> > Signed-off-by: Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org>
>> > ---
>> >  .../devicetree/bindings/spi/jcore,spi.txt          | 30 ++++++++++++++++++++++
>> >  1 file changed, 30 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Thanks. Since I switched to to using the clk framework rather than a
> clock-frequency property (at Mark Brown's suggestion/request), I'll be
> submitting one more version for your review.
>
> After that, who should be responsible for upstreaming this and the
> other binding patches? You, me, or the subsystem maintainers that the
> drivers are going through?

Mark will take the SPI changes. I generally only take standalone
binding patches thru the DT tree.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html