Linux SPI subsystem development
 help / color / mirror / Atom feed
* [PATCH v3 27/27] devres: kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-23 11:02 UTC (permalink / raw)
  To: linux-kernel, gregkh
  Cc: ysxie, ulf.hansson, linux-mmc, boris.brezillon, richard,
	marek.vasut, cyrille.pitchen, linux-mtd, alsa-devel, wim, linux,
	linux-watchdog, b.zolnierkie, linux-fbdev, linus.walleij,
	linux-gpio, ralf, linux-mips, lgirdwood, broonie, tglx, jason,
	marc.zyngier, arnd, andriy.shevchenko, industrypack-devel, wg,
	mkl, linux-can, <mcheh

Now, nobody use devm_ioremap_nocache anymore, can it can just be
removed. After this patch the size of devres.o will be reduced from
20304 bytes to 18992 bytes.

Suggested-by: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 Documentation/driver-model/devres.txt   |  1 -
 include/linux/io.h                      |  2 --
 lib/devres.c                            | 29 -----------------------------
 scripts/coccinelle/free/devm_free.cocci |  2 --
 4 files changed, 34 deletions(-)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index c180045..c3fddb5 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -292,7 +292,6 @@ IOMAP
   devm_ioport_map()
   devm_ioport_unmap()
   devm_ioremap()
-  devm_ioremap_nocache()
   devm_ioremap_wc()
   devm_ioremap_resource() : checks resource, requests memory region, ioremaps
   devm_iounmap()
diff --git a/include/linux/io.h b/include/linux/io.h
index 32e30e8..a9c7270 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -75,8 +75,6 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
 
 void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
 			   resource_size_t size);
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
-				   resource_size_t size);
 void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
 				   resource_size_t size);
 void devm_iounmap(struct device *dev, void __iomem *addr);
diff --git a/lib/devres.c b/lib/devres.c
index 5f2aedd..f818fcf 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -44,35 +44,6 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
 EXPORT_SYMBOL(devm_ioremap);
 
 /**
- * devm_ioremap_nocache - Managed ioremap_nocache()
- * @dev: Generic device to remap IO address for
- * @offset: Resource address to map
- * @size: Size of map
- *
- * Managed ioremap_nocache().  Map is automatically unmapped on driver
- * detach.
- */
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
-				   resource_size_t size)
-{
-	void __iomem **ptr, *addr;
-
-	ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return NULL;
-
-	addr = ioremap_nocache(offset, size);
-	if (addr) {
-		*ptr = addr;
-		devres_add(dev, ptr);
-	} else
-		devres_free(ptr);
-
-	return addr;
-}
-EXPORT_SYMBOL(devm_ioremap_nocache);
-
-/**
  * devm_ioremap_wc - Managed ioremap_wc()
  * @dev: Generic device to remap IO address for
  * @offset: Resource address to map
diff --git a/scripts/coccinelle/free/devm_free.cocci b/scripts/coccinelle/free/devm_free.cocci
index c990d2c..36b8752 100644
--- a/scripts/coccinelle/free/devm_free.cocci
+++ b/scripts/coccinelle/free/devm_free.cocci
@@ -51,8 +51,6 @@ expression x;
 |
  x = devm_ioremap(...)
 |
- x = devm_ioremap_nocache(...)
-|
  x = devm_ioport_map(...)
 )
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 02/27] spi: replace devm_ioremap_nocache with devm_ioremap
From: Yisheng Xie @ 2017-12-23 10:56 UTC (permalink / raw)
  To: linux-kernel, gregkh; +Cc: ysxie, Yisheng Xie, Mark Brown, linux-spi

Default ioremap is ioremap_nocache, so devm_ioremap has the same
function with devm_ioremap_nocache, which can just be killed to
save the size of devres.o.

This patch is to use use devm_ioremap instead of devm_ioremap_nocache,
which should not have any function change but prepare for killing
devm_ioremap_nocache.

Cc: Mark Brown <broonie@kernel.org>
Cc: linux-spi@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/spi/spi-jcore.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spi/spi-jcore.c b/drivers/spi/spi-jcore.c
index cebfea5..32f538e 100644
--- a/drivers/spi/spi-jcore.c
+++ b/drivers/spi/spi-jcore.c
@@ -169,8 +169,7 @@ static int jcore_spi_probe(struct platform_device *pdev)
 	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));
+	hw->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
 	if (!hw->base)
 		goto exit_busy;
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v3 00/27] kill devm_ioremap_nocache
From: Yisheng Xie @ 2017-12-23 10:55 UTC (permalink / raw)
  To: linux-kernel, gregkh
  Cc: ysxie, ulf.hansson, linux-mmc, boris.brezillon, richard,
	marek.vasut, cyrille.pitchen, linux-mtd, alsa-devel, wim, linux,
	linux-watchdog, b.zolnierkie, linux-fbdev, linus.walleij,
	linux-gpio, ralf, linux-mips, lgirdwood, broonie, tglx, jason,
	marc.zyngier, arnd, andriy.shevchenko, industrypack-devel, wg,
	mkl, linux-can, <mcheh

Hi all,

When I tried to use devm_ioremap function and review related code, I found
devm_ioremap and devm_ioremap_nocache is almost the same with each other,
except one use ioremap while the other use ioremap_nocache. While ioremap's
default function is ioremap_nocache, so devm_ioremap_nocache also have the
same function with devm_ioremap, which can just be killed to reduce the size
of devres.o(from 20304 bytes to 18992 bytes in my compile environment).

I have posted two versions, which use macro instead of function for
devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
devm_ioremap_nocache for no need to keep a macro around for the duplicate
thing. So here comes v3 and please help to review.

Thanks so much!
Yisheng Xie

[1] https://lkml.org/lkml/2017/11/20/135
[2] https://lkml.org/lkml/2017/11/25/21

Yisheng Xie (27):
  ASOC: replace devm_ioremap_nocache with devm_ioremap
  spi: replace devm_ioremap_nocache with devm_ioremap
  staging: replace devm_ioremap_nocache with devm_ioremap
  ipack: replace devm_ioremap_nocache with devm_ioremap
  media: replace devm_ioremap_nocache with devm_ioremap
  gpio: replace devm_ioremap_nocache with devm_ioremap
  mmc: replace devm_ioremap_nocache with devm_ioremap
  PCI: replace devm_ioremap_nocache with devm_ioremap
  platform/x86: replace devm_ioremap_nocache with devm_ioremap
  tty: replace devm_ioremap_nocache with devm_ioremap
  video: replace devm_ioremap_nocache with devm_ioremap
  rtc: replace devm_ioremap_nocache with devm_ioremap
  char: replace devm_ioremap_nocache with devm_ioremap
  mtd: nand: replace devm_ioremap_nocache with devm_ioremap
  dmaengine: replace devm_ioremap_nocache with devm_ioremap
  ata: replace devm_ioremap_nocache with devm_ioremap
  irqchip: replace devm_ioremap_nocache with devm_ioremap
  pinctrl: replace devm_ioremap_nocache with devm_ioremap
  drm: replace devm_ioremap_nocache with devm_ioremap
  regulator: replace devm_ioremap_nocache with devm_ioremap
  watchdog: replace devm_ioremap_nocache with devm_ioremap
  tools/testing/nvdimm: replace devm_ioremap_nocache with devm_ioremap
  MIPS: pci: replace devm_ioremap_nocache with devm_ioremap
  can: replace devm_ioremap_nocache with devm_ioremap
  wireless: replace devm_ioremap_nocache with devm_ioremap
  ethernet: replace devm_ioremap_nocache with devm_ioremap
  devres: kill devm_ioremap_nocache

 Documentation/driver-model/devres.txt           |  1 -
 arch/mips/pci/pci-ar2315.c                      |  3 +--
 drivers/ata/pata_arasan_cf.c                    |  3 +--
 drivers/ata/pata_octeon_cf.c                    |  9 ++++----
 drivers/ata/pata_rb532_cf.c                     |  2 +-
 drivers/char/hw_random/bcm63xx-rng.c            |  3 +--
 drivers/char/hw_random/octeon-rng.c             | 10 ++++-----
 drivers/dma/altera-msgdma.c                     |  3 +--
 drivers/dma/sprd-dma.c                          |  4 ++--
 drivers/gpio/gpio-ath79.c                       |  3 +--
 drivers/gpio/gpio-em.c                          |  6 ++---
 drivers/gpio/gpio-htc-egpio.c                   |  4 ++--
 drivers/gpio/gpio-xgene.c                       |  3 +--
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c                   |  2 +-
 drivers/gpu/drm/sti/sti_dvo.c                   |  3 +--
 drivers/gpu/drm/sti/sti_hda.c                   |  4 ++--
 drivers/gpu/drm/sti/sti_hdmi.c                  |  2 +-
 drivers/gpu/drm/sti/sti_tvout.c                 |  2 +-
 drivers/gpu/drm/sti/sti_vtg.c                   |  2 +-
 drivers/ipack/devices/ipoctal.c                 | 13 +++++------
 drivers/irqchip/irq-renesas-intc-irqpin.c       |  4 ++--
 drivers/media/platform/tegra-cec/tegra_cec.c    |  4 ++--
 drivers/mmc/host/sdhci-acpi.c                   |  3 +--
 drivers/mtd/nand/fsl_upm.c                      |  4 ++--
 drivers/net/can/sja1000/sja1000_platform.c      |  4 ++--
 drivers/net/ethernet/altera/altera_tse_main.c   |  3 +--
 drivers/net/ethernet/ethoc.c                    |  8 +++----
 drivers/net/ethernet/lantiq_etop.c              |  4 ++--
 drivers/net/ethernet/ti/netcp_core.c            |  2 +-
 drivers/net/wireless/ath/ath9k/ahb.c            |  2 +-
 drivers/pci/dwc/pci-dra7xx.c                    |  2 +-
 drivers/pinctrl/bcm/pinctrl-ns2-mux.c           |  2 +-
 drivers/pinctrl/bcm/pinctrl-nsp-mux.c           |  4 ++--
 drivers/pinctrl/freescale/pinctrl-imx1-core.c   |  2 +-
 drivers/pinctrl/pinctrl-amd.c                   |  4 ++--
 drivers/platform/x86/intel_pmc_core.c           |  5 ++---
 drivers/regulator/ti-abb-regulator.c            |  6 ++---
 drivers/rtc/rtc-sh.c                            |  4 ++--
 drivers/spi/spi-jcore.c                         |  3 +--
 drivers/staging/fsl-mc/bus/mc-io.c              |  8 +++----
 drivers/tty/mips_ejtag_fdc.c                    |  4 ++--
 drivers/tty/serial/8250/8250_omap.c             |  3 +--
 drivers/tty/serial/lantiq.c                     |  3 +--
 drivers/tty/serial/meson_uart.c                 |  3 +--
 drivers/tty/serial/owl-uart.c                   |  2 +-
 drivers/tty/serial/pic32_uart.c                 |  4 ++--
 drivers/video/fbdev/mbx/mbxfb.c                 |  9 ++++----
 drivers/video/fbdev/mmp/hw/mmp_ctrl.c           |  2 +-
 drivers/video/fbdev/pxa168fb.c                  |  4 ++--
 drivers/watchdog/bcm63xx_wdt.c                  |  4 ++--
 drivers/watchdog/rc32434_wdt.c                  |  4 ++--
 include/linux/io.h                              |  2 --
 lib/devres.c                                    | 29 -------------------------
 scripts/coccinelle/free/devm_free.cocci         |  2 --
 sound/soc/au1x/ac97c.c                          |  4 ++--
 sound/soc/au1x/i2sc.c                           |  4 ++--
 sound/soc/intel/atom/sst/sst_acpi.c             | 20 ++++++++---------
 sound/soc/intel/boards/mfld_machine.c           |  4 ++--
 sound/soc/sh/fsi.c                              |  4 ++--
 tools/testing/nvdimm/Kbuild                     |  2 +-
 tools/testing/nvdimm/test/iomap.c               |  2 +-
 62 files changed, 109 insertions(+), 168 deletions(-)

-- 
1.8.3.1


^ permalink raw reply

* Re: [PATCH] spi: pxa2xx: Use gpiod_put() not gpiod_free()
From: Geert Uytterhoeven @ 2017-12-23  9:01 UTC (permalink / raw)
  To: Mark Brown; +Cc: Stephen Rothwell, Rasmus Villemoes, linux-spi
In-Reply-To: <20171222161755.2742-1-broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Hi Mark,

On Fri, Dec 22, 2017 at 5:17 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> gpiod_free() is an internal function for gpiolib, gpiod_put() is the
> correct external function.
>
> Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> Suggested-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
> Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Given you usually don't rebase spi/for-next:
Fixes: 221886646f75964c ("spi: pxa2xx: avoid redundant
gpio_to_desc(desc_to_gpio()) round-trip")

Else we may have to repeat the exercise for stable later ;-)

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

^ permalink raw reply

* Re: [PATCH] spi: Add a sysfs interface to instantiate devices
From: Geert Uytterhoeven @ 2017-12-23  8:58 UTC (permalink / raw)
  To: Kyle Roeschley
  Cc: Mark Brown, Trent Piepho,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20171222171108.GA31574@senary>

Hi Kyle,

On Fri, Dec 22, 2017 at 6:11 PM, Kyle Roeschley <kyle.roeschley-acOepvfBmUk@public.gmane.org> wrote:
> On Fri, Dec 22, 2017 at 03:56:03PM +0000, Mark Brown wrote:
>> On Thu, Dec 21, 2017 at 09:05:43PM +0000, Trent Piepho wrote:
>> > On Thu, 2017-12-21 at 14:03 -0600, Kyle Roeschley wrote:
>> > > Add a sysfs interface to instantiate and delete SPI devices using the
>> > > spidev driver. This can be used when developing a driver on a
>> > > self-soldered board which doesn't yet have proper SPI device declaration
>> > > at the platform level, and presumably for various debugging situations.
>>
>> > > Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
>> > > devices").
>>
>> > The i2c interface allows one to specify the type of device to create.
>> > Why must this interface be linked to spidev and only capable of
>> > creating spidev devices?
>>
>> Right, that doesn't seem good.  I also can't see anything in the actual
>> code which suggests that this is tied to spidev except the log messages.
>
> Quoting Geert's email [1] on the subject:
>
>> To me, the above sounds a bit contradictive: either you have
>>   1. a simple (trivial) description, which can be handled by spidev and
>>      userspace, and thus by just writing "<unit-addr> spidev" to a new_device

Note the "spidev" in the string written...

>>      sysfs node, or
>>   2. a complex description, for which you need a specialized in-kernel driver,
>>      so you're gonna need a real DT node (and overlays?) to describe it.
>>
>> I don't think writing a complex description to a new_device sysfs node makes
>> sense.
>
> And regarding not being linked to spidev, see modalias in new_device_store:
>
>> > > + struct spi_board_info bi = {
>> > > +         .modalias = "spidev",

I would make it a little bit more generic and extract the modalias from the
string written.

>> > > +         .max_speed_hz = ctlr->max_speed_hz,
>> > > + };
>
> [1] https://marc.info/?l=linux-spi&m=151199390921251&w=2

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

^ permalink raw reply

* Re: [PATCH] spi: Add a sysfs interface to instantiate devices
From: Kyle Roeschley @ 2017-12-22 17:11 UTC (permalink / raw)
  To: Mark Brown
  Cc: Trent Piepho, linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In-Reply-To: <20171222155603.GM1827@finisterre>

On Fri, Dec 22, 2017 at 03:56:03PM +0000, Mark Brown wrote:
> On Thu, Dec 21, 2017 at 09:05:43PM +0000, Trent Piepho wrote:
> > On Thu, 2017-12-21 at 14:03 -0600, Kyle Roeschley wrote:
> > > Add a sysfs interface to instantiate and delete SPI devices using the
> > > spidev driver. This can be used when developing a driver on a
> > > self-soldered board which doesn't yet have proper SPI device declaration
> > > at the platform level, and presumably for various debugging situations.
> 
> > > Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
> > > devices").
> 
> > The i2c interface allows one to specify the type of device to create.
> > Why must this interface be linked to spidev and only capable of
> > creating spidev devices?
> 
> Right, that doesn't seem good.  I also can't see anything in the actual
> code which suggests that this is tied to spidev except the log messages.
> 

Quoting Geert's email [1] on the subject:

> To me, the above sounds a bit contradictive: either you have
>   1. a simple (trivial) description, which can be handled by spidev and
>      userspace, and thus by just writing "<unit-addr> spidev" to a new_device
>      sysfs node, or
>   2. a complex description, for which you need a specialized in-kernel driver,
>      so you're gonna need a real DT node (and overlays?) to describe it.
> 
> I don't think writing a complex description to a new_device sysfs node makes
> sense.

And regarding not being linked to spidev, see modalias in new_device_store:

> > > +	struct spi_board_info bi = {
> > > +		.modalias = "spidev",
> > > +		.max_speed_hz = ctlr->max_speed_hz,
> > > +	};

[1] https://marc.info/?l=linux-spi&m=151199390921251&w=2

Happy holidays,

-- 
Kyle Roeschley
Software Engineer
National Instruments
--
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

^ permalink raw reply

* [PATCH] spi: pxa2xx: Use gpiod_put() not gpiod_free()
From: Mark Brown @ 2017-12-22 16:17 UTC (permalink / raw)
  To: Stephen Rothwell, Rasmus Villemoes
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Mark Brown

gpiod_free() is an internal function for gpiolib, gpiod_put() is the
correct external function.

Reported-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Suggested-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-pxa2xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index c209dc1047b5..b0822d1dba29 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1237,7 +1237,7 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 	 * different chip_info, release previously requested GPIO
 	 */
 	if (chip->gpiod_cs) {
-		gpiod_free(chip->gpiod_cs);
+		gpiod_put(chip->gpiod_cs);
 		chip->gpiod_cs = NULL;
 	}
 
@@ -1417,7 +1417,7 @@ static void cleanup(struct spi_device *spi)
 
 	if (drv_data->ssp_type != CE4100_SSP && !drv_data->cs_gpiods &&
 	    chip->gpiod_cs)
-		gpiod_free(chip->gpiod_cs);
+		gpiod_put(chip->gpiod_cs);
 
 	kfree(chip);
 }
-- 
2.15.0

--
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

^ permalink raw reply related

* Re: [PATCH] spi: Add a sysfs interface to instantiate devices
From: Mark Brown @ 2017-12-22 15:56 UTC (permalink / raw)
  To: Trent Piepho
  Cc: kyle.roeschley-acOepvfBmUk@public.gmane.org,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In-Reply-To: <1513890342.26695.4.camel-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org>

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

On Thu, Dec 21, 2017 at 09:05:43PM +0000, Trent Piepho wrote:
> On Thu, 2017-12-21 at 14:03 -0600, Kyle Roeschley wrote:
> > Add a sysfs interface to instantiate and delete SPI devices using the
> > spidev driver. This can be used when developing a driver on a
> > self-soldered board which doesn't yet have proper SPI device declaration
> > at the platform level, and presumably for various debugging situations.

> > Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
> > devices").

> The i2c interface allows one to specify the type of device to create. 
> Why must this interface be linked to spidev and only capable of
> creating spidev devices?  

Right, that doesn't seem good.  I also can't see anything in the actual
code which suggests that this is tied to spidev except the log messages.

> > +						   dev);
> > +	struct spi_device *spi, *next;
> > +	int ret = -ENXIO;
> > +	u16 cs;

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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

^ permalink raw reply

* [spi:topic/pxa2xx 1/1] drivers//spi/spi-pxa2xx.c:1240:3: error: implicit declaration of function 'gpiod_free'
From: kbuild test robot @ 2017-12-22 10:29 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: kbuild-all-JC7UmRfGjtg, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	Mark Brown

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git topic/pxa2xx
head:   221886646f75964ca31cf60f1811b2c9c4e965a5
commit: 221886646f75964ca31cf60f1811b2c9c4e965a5 [1/1] spi: pxa2xx: avoid redundant gpio_to_desc(desc_to_gpio()) round-trip
config: x86_64-randconfig-r0-12221401 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        git checkout 221886646f75964ca31cf60f1811b2c9c4e965a5
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers//spi/spi-pxa2xx.c: In function 'setup_cs':
>> drivers//spi/spi-pxa2xx.c:1240:3: error: implicit declaration of function 'gpiod_free' [-Werror=implicit-function-declaration]
      gpiod_free(chip->gpiod_cs);
      ^~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/gpiod_free +1240 drivers//spi/spi-pxa2xx.c

  1211	
  1212	static int setup_cs(struct spi_device *spi, struct chip_data *chip,
  1213			    struct pxa2xx_spi_chip *chip_info)
  1214	{
  1215		struct driver_data *drv_data = spi_master_get_devdata(spi->master);
  1216		struct gpio_desc *gpiod;
  1217		int err = 0;
  1218	
  1219		if (chip == NULL)
  1220			return 0;
  1221	
  1222		if (drv_data->cs_gpiods) {
  1223			gpiod = drv_data->cs_gpiods[spi->chip_select];
  1224			if (gpiod) {
  1225				chip->gpiod_cs = gpiod;
  1226				chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
  1227				gpiod_set_value(gpiod, chip->gpio_cs_inverted);
  1228			}
  1229	
  1230			return 0;
  1231		}
  1232	
  1233		if (chip_info == NULL)
  1234			return 0;
  1235	
  1236		/* NOTE: setup() can be called multiple times, possibly with
  1237		 * different chip_info, release previously requested GPIO
  1238		 */
  1239		if (chip->gpiod_cs) {
> 1240			gpiod_free(chip->gpiod_cs);
  1241			chip->gpiod_cs = NULL;
  1242		}
  1243	
  1244		/* If (*cs_control) is provided, ignore GPIO chip select */
  1245		if (chip_info->cs_control) {
  1246			chip->cs_control = chip_info->cs_control;
  1247			return 0;
  1248		}
  1249	
  1250		if (gpio_is_valid(chip_info->gpio_cs)) {
  1251			err = gpio_request(chip_info->gpio_cs, "SPI_CS");
  1252			if (err) {
  1253				dev_err(&spi->dev, "failed to request chip select GPIO%d\n",
  1254					chip_info->gpio_cs);
  1255				return err;
  1256			}
  1257	
  1258			gpiod = gpio_to_desc(chip_info->gpio_cs);
  1259			chip->gpiod_cs = gpiod;
  1260			chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
  1261	
  1262			err = gpiod_direction_output(gpiod, !chip->gpio_cs_inverted);
  1263		}
  1264	
  1265		return err;
  1266	}
  1267	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25008 bytes --]

^ permalink raw reply

* Re: [PATCH] spi: Add a sysfs interface to instantiate devices
From: Trent Piepho @ 2017-12-21 21:05 UTC (permalink / raw)
  To: kyle.roeschley-acOepvfBmUk@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In-Reply-To: <20171221200309.17967-1-kyle.roeschley-acOepvfBmUk@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6174 bytes --]

On Thu, 2017-12-21 at 14:03 -0600, Kyle Roeschley wrote:
> Add a sysfs interface to instantiate and delete SPI devices using the
> spidev driver. This can be used when developing a driver on a
> self-soldered board which doesn't yet have proper SPI device declaration
> at the platform level, and presumably for various debugging situations.
> 
> Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
> devices").

The i2c interface allows one to specify the type of device to create. 
Why must this interface be linked to spidev and only capable of
creating spidev devices?  

> 
> Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com>
> ---
>  Documentation/spi/spi-summary | 14 ++++++++
>  drivers/spi/spi.c             | 78 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/spi/spi.h       |  3 ++
>  3 files changed, 95 insertions(+)
> 
> diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
> index 1721c1b570c3..51d9747c4426 100644
> --- a/Documentation/spi/spi-summary
> +++ b/Documentation/spi/spi-summary
> @@ -339,6 +339,20 @@ up the spi bus master, and will likely need spi_new_device() to provide the
>  board info based on the board that was hotplugged.  Of course, you'd later
>  call at least spi_unregister_device() when that board is removed.
>  
> +Alternatively, a sysfs interface was added to let the user create devices which
> +using the spidev driver. This interface is made of 2 attribute files which are
> +created in every SPI master directory: new_device and delete_device. Both files
> +are write only and you must write the decimal SPI chip select number to them in
> +order to properly instantiate or delete a SPI device. As no two devices can be
> +attached to the same master with the same chip select line, the chip select
> +number is sufficient to uniquely identify the device to be deleted.
> +
> +Example:
> +# echo 1 > /sys/class/spi_master/spi0/new_device
> +
> +In general, this interface should only be used when in-kernel device
> +declaration can't be done.
> +
>  When Linux includes support for MMC/SD/SDIO/DataFlash cards through SPI, those
>  configurations will also be dynamic.  Fortunately, such devices all support
>  basic device identification probes, so they should hotplug normally.
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index b33a727a0158..648ccdf359f9 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -242,8 +242,85 @@ static const struct attribute_group spi_controller_statistics_group = {
>  	.attrs  = spi_controller_statistics_attrs,
>  };
>  
> +static ssize_t
> +new_device_store(struct device *dev, struct device_attribute *attr,
> +		 const char *buf, size_t count)
> +{
> +	struct spi_controller *ctlr = container_of(dev, struct spi_controller,
> +						   dev);
> +	struct spi_device *spi;
> +	struct spi_board_info bi = {
> +		.modalias = "spidev",
> +		.max_speed_hz = ctlr->max_speed_hz,
> +	};
> +
> +	if (kstrtou16(buf, 0, &bi.chip_select) < 0)
> +		return -EINVAL;
> +
> +	spi = spi_new_device(ctlr, &bi);
> +	if (!spi) {
> +		dev_err(dev, "can't create new device\n");
> +		return -ENXIO;

I2C returns -EINVAL

> +	}
> +
> +	mutex_lock(&ctlr->bus_lock_mutex);
> +	list_add_tail(&spi->userspace_device, &ctlr->userspace_devices);
> +	mutex_unlock(&ctlr->bus_lock_mutex);
> +
> +	dev_info(dev, "created spidev device %s\n", dev_name(&spi->dev));
> +
> +	return count;
> +}
> +static DEVICE_ATTR_WO(new_device);
> +
> +static ssize_t
> +delete_device_store(struct device *dev, struct device_attribute *attr,
> +		    const char *buf, size_t count)
> +{
> +	struct spi_controller *ctlr = container_of(dev, struct spi_controller,
> +						   dev);
> +	struct spi_device *spi, *next;
> +	int ret = -ENXIO;
> +	u16 cs;
> +
> +	if (kstrtou16(buf, 0, &cs) < 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&ctlr->bus_lock_mutex);
> +	list_for_each_entry_safe(spi, next, &ctlr->userspace_devices,
> +				 userspace_device) {
> +		if (spi->chip_select != cs)
> +			continue;
> +
> +		dev_info(dev, "deleting spidev device %s\n",
> +			 dev_name(&spi->dev));
> +		list_del(&spi->userspace_device);
> +		spi_unregister_device(spi);
> +		ret = count;
> +		break;
> +	}
> +	mutex_unlock(&ctlr->bus_lock_mutex);
> +
> +	if (ret == -ENXIO)
> +		dev_err(dev, "can't find spidev device %u in list\n", cs);
> +
> +	return ret;
> +}
> +static DEVICE_ATTR_WO(delete_device);
> +
> +static struct attribute *spi_controller_userspace_attrs[] = {
> +	&dev_attr_new_device.attr,
> +	&dev_attr_delete_device.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group spi_controller_userspace_group = {
> +	.attrs  = spi_controller_userspace_attrs,
> +};
> +
>  static const struct attribute_group *spi_master_groups[] = {
>  	&spi_controller_statistics_group,
> +	&spi_controller_userspace_group,
>  	NULL,
>  };
>  
> @@ -2129,6 +2206,7 @@ int spi_register_controller(struct spi_controller *ctlr)
>  			return id;
>  		ctlr->bus_num = id;
>  	}
> +	INIT_LIST_HEAD(&ctlr->userspace_devices);
>  	INIT_LIST_HEAD(&ctlr->queue);
>  	spin_lock_init(&ctlr->queue_lock);
>  	spin_lock_init(&ctlr->bus_lock_spinlock);
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index bc6bb325d1bf..f7255745326d 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -172,6 +172,8 @@ struct spi_device {
>  	/* the statistics */
>  	struct spi_statistics	statistics;
>  
> +	struct list_head	userspace_device;
> +
>  	/*
>  	 * likely need more hooks for more protocol options affecting how
>  	 * the controller talks to each chip, like:
> @@ -410,6 +412,7 @@ struct spi_controller {
>  	struct device	dev;
>  
>  	struct list_head list;
> +	struct list_head userspace_devices;
>  
>  	/* other than negative (== assign one dynamically), bus_num is fully
>  	 * board-specific.  usually that simplifies to being SOC-specific.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±²˜¢žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

^ permalink raw reply

* Re: [PATCH] spi: Add a sysfs interface to instantiate devices
From: Randy Dunlap @ 2017-12-21 20:11 UTC (permalink / raw)
  To: Kyle Roeschley, Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven
In-Reply-To: <20171221200309.17967-1-kyle.roeschley-acOepvfBmUk@public.gmane.org>

On 12/21/2017 12:03 PM, Kyle Roeschley wrote:
> Add a sysfs interface to instantiate and delete SPI devices using the
> spidev driver. This can be used when developing a driver on a
> self-soldered board which doesn't yet have proper SPI device declaration
> at the platform level, and presumably for various debugging situations.
> 
> Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
> devices").
> 
> Signed-off-by: Kyle Roeschley <kyle.roeschley-acOepvfBmUk@public.gmane.org>
> ---
>  Documentation/spi/spi-summary | 14 ++++++++
>  drivers/spi/spi.c             | 78 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/spi/spi.h       |  3 ++
>  3 files changed, 95 insertions(+)
> 
> diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
> index 1721c1b570c3..51d9747c4426 100644
> --- a/Documentation/spi/spi-summary
> +++ b/Documentation/spi/spi-summary
> @@ -339,6 +339,20 @@ up the spi bus master, and will likely need spi_new_device() to provide the
>  board info based on the board that was hotplugged.  Of course, you'd later
>  call at least spi_unregister_device() when that board is removed.
>  
> +Alternatively, a sysfs interface was added to let the user create devices which

                                                                             when

> +using the spidev driver. This interface is made of 2 attribute files which are
> +created in every SPI master directory: new_device and delete_device. Both files
> +are write only and you must write the decimal SPI chip select number to them in

       write-only

> +order to properly instantiate or delete a SPI device. As no two devices can be
> +attached to the same master with the same chip select line, the chip select
> +number is sufficient to uniquely identify the device to be deleted.
> +
> +Example:
> +# echo 1 > /sys/class/spi_master/spi0/new_device
> +
> +In general, this interface should only be used when in-kernel device
> +declaration can't be done.
> +
>  When Linux includes support for MMC/SD/SDIO/DataFlash cards through SPI, those
>  configurations will also be dynamic.  Fortunately, such devices all support
>  basic device identification probes, so they should hotplug normally.

thanks.
-- 
~Randy
--
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

^ permalink raw reply

* [PATCH] spi: Add a sysfs interface to instantiate devices
From: Kyle Roeschley @ 2017-12-21 20:03 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven

Add a sysfs interface to instantiate and delete SPI devices using the
spidev driver. This can be used when developing a driver on a
self-soldered board which doesn't yet have proper SPI device declaration
at the platform level, and presumably for various debugging situations.

Inspired by 99cd8e25875a ("i2c: Add a sysfs interface to instantiate
devices").

Signed-off-by: Kyle Roeschley <kyle.roeschley-acOepvfBmUk@public.gmane.org>
---
 Documentation/spi/spi-summary | 14 ++++++++
 drivers/spi/spi.c             | 78 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi.h       |  3 ++
 3 files changed, 95 insertions(+)

diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary
index 1721c1b570c3..51d9747c4426 100644
--- a/Documentation/spi/spi-summary
+++ b/Documentation/spi/spi-summary
@@ -339,6 +339,20 @@ up the spi bus master, and will likely need spi_new_device() to provide the
 board info based on the board that was hotplugged.  Of course, you'd later
 call at least spi_unregister_device() when that board is removed.
 
+Alternatively, a sysfs interface was added to let the user create devices which
+using the spidev driver. This interface is made of 2 attribute files which are
+created in every SPI master directory: new_device and delete_device. Both files
+are write only and you must write the decimal SPI chip select number to them in
+order to properly instantiate or delete a SPI device. As no two devices can be
+attached to the same master with the same chip select line, the chip select
+number is sufficient to uniquely identify the device to be deleted.
+
+Example:
+# echo 1 > /sys/class/spi_master/spi0/new_device
+
+In general, this interface should only be used when in-kernel device
+declaration can't be done.
+
 When Linux includes support for MMC/SD/SDIO/DataFlash cards through SPI, those
 configurations will also be dynamic.  Fortunately, such devices all support
 basic device identification probes, so they should hotplug normally.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b33a727a0158..648ccdf359f9 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -242,8 +242,85 @@ static const struct attribute_group spi_controller_statistics_group = {
 	.attrs  = spi_controller_statistics_attrs,
 };
 
+static ssize_t
+new_device_store(struct device *dev, struct device_attribute *attr,
+		 const char *buf, size_t count)
+{
+	struct spi_controller *ctlr = container_of(dev, struct spi_controller,
+						   dev);
+	struct spi_device *spi;
+	struct spi_board_info bi = {
+		.modalias = "spidev",
+		.max_speed_hz = ctlr->max_speed_hz,
+	};
+
+	if (kstrtou16(buf, 0, &bi.chip_select) < 0)
+		return -EINVAL;
+
+	spi = spi_new_device(ctlr, &bi);
+	if (!spi) {
+		dev_err(dev, "can't create new device\n");
+		return -ENXIO;
+	}
+
+	mutex_lock(&ctlr->bus_lock_mutex);
+	list_add_tail(&spi->userspace_device, &ctlr->userspace_devices);
+	mutex_unlock(&ctlr->bus_lock_mutex);
+
+	dev_info(dev, "created spidev device %s\n", dev_name(&spi->dev));
+
+	return count;
+}
+static DEVICE_ATTR_WO(new_device);
+
+static ssize_t
+delete_device_store(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	struct spi_controller *ctlr = container_of(dev, struct spi_controller,
+						   dev);
+	struct spi_device *spi, *next;
+	int ret = -ENXIO;
+	u16 cs;
+
+	if (kstrtou16(buf, 0, &cs) < 0)
+		return -EINVAL;
+
+	mutex_lock(&ctlr->bus_lock_mutex);
+	list_for_each_entry_safe(spi, next, &ctlr->userspace_devices,
+				 userspace_device) {
+		if (spi->chip_select != cs)
+			continue;
+
+		dev_info(dev, "deleting spidev device %s\n",
+			 dev_name(&spi->dev));
+		list_del(&spi->userspace_device);
+		spi_unregister_device(spi);
+		ret = count;
+		break;
+	}
+	mutex_unlock(&ctlr->bus_lock_mutex);
+
+	if (ret == -ENXIO)
+		dev_err(dev, "can't find spidev device %u in list\n", cs);
+
+	return ret;
+}
+static DEVICE_ATTR_WO(delete_device);
+
+static struct attribute *spi_controller_userspace_attrs[] = {
+	&dev_attr_new_device.attr,
+	&dev_attr_delete_device.attr,
+	NULL,
+};
+
+static const struct attribute_group spi_controller_userspace_group = {
+	.attrs  = spi_controller_userspace_attrs,
+};
+
 static const struct attribute_group *spi_master_groups[] = {
 	&spi_controller_statistics_group,
+	&spi_controller_userspace_group,
 	NULL,
 };
 
@@ -2129,6 +2206,7 @@ int spi_register_controller(struct spi_controller *ctlr)
 			return id;
 		ctlr->bus_num = id;
 	}
+	INIT_LIST_HEAD(&ctlr->userspace_devices);
 	INIT_LIST_HEAD(&ctlr->queue);
 	spin_lock_init(&ctlr->queue_lock);
 	spin_lock_init(&ctlr->bus_lock_spinlock);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index bc6bb325d1bf..f7255745326d 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -172,6 +172,8 @@ struct spi_device {
 	/* the statistics */
 	struct spi_statistics	statistics;
 
+	struct list_head	userspace_device;
+
 	/*
 	 * likely need more hooks for more protocol options affecting how
 	 * the controller talks to each chip, like:
@@ -410,6 +412,7 @@ struct spi_controller {
 	struct device	dev;
 
 	struct list_head list;
+	struct list_head userspace_devices;
 
 	/* other than negative (== assign one dynamically), bus_num is fully
 	 * board-specific.  usually that simplifies to being SOC-specific.
-- 
2.15.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

^ permalink raw reply related

* Applied "spi: pxa2xx: avoid redundant gpio_to_desc(desc_to_gpio()) round-trip" to the spi tree
From: Mark Brown @ 2017-12-21 12:28 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Mark Brown, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
	Mark Brown, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171221003731.21633-1-linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>

The patch

   spi: pxa2xx: avoid redundant gpio_to_desc(desc_to_gpio()) round-trip

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 221886646f75964ca31cf60f1811b2c9c4e965a5 Mon Sep 17 00:00:00 2001
From: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
Date: Thu, 21 Dec 2017 01:37:31 +0100
Subject: [PATCH] spi: pxa2xx: avoid redundant gpio_to_desc(desc_to_gpio())
 round-trip

gpio_free(gpio) simply does gpiod_free(gpio_to_desc(gpio)), so it's
simpler and cleaner to use gpiod_free directly.

Signed-off-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-pxa2xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 4cb515a3104c..c209dc1047b5 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1237,7 +1237,7 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 	 * different chip_info, release previously requested GPIO
 	 */
 	if (chip->gpiod_cs) {
-		gpio_free(desc_to_gpio(chip->gpiod_cs));
+		gpiod_free(chip->gpiod_cs);
 		chip->gpiod_cs = NULL;
 	}
 
@@ -1417,7 +1417,7 @@ static void cleanup(struct spi_device *spi)
 
 	if (drv_data->ssp_type != CE4100_SSP && !drv_data->cs_gpiods &&
 	    chip->gpiod_cs)
-		gpio_free(desc_to_gpio(chip->gpiod_cs));
+		gpiod_free(chip->gpiod_cs);
 
 	kfree(chip);
 }
-- 
2.15.0

--
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

^ permalink raw reply related

* [PATCH] spi: pxa2xx: avoid redundant gpio_to_desc(desc_to_gpio()) round-trip
From: Rasmus Villemoes @ 2017-12-21  0:37 UTC (permalink / raw)
  To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Mark Brown
  Cc: Rasmus Villemoes,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

gpio_free(gpio) simply does gpiod_free(gpio_to_desc(gpio)), so it's
simpler and cleaner to use gpiod_free directly.

Signed-off-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
---
 drivers/spi/spi-pxa2xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 4cb515a3104c..c209dc1047b5 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1237,7 +1237,7 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 	 * different chip_info, release previously requested GPIO
 	 */
 	if (chip->gpiod_cs) {
-		gpio_free(desc_to_gpio(chip->gpiod_cs));
+		gpiod_free(chip->gpiod_cs);
 		chip->gpiod_cs = NULL;
 	}
 
@@ -1417,7 +1417,7 @@ static void cleanup(struct spi_device *spi)
 
 	if (drv_data->ssp_type != CE4100_SSP && !drv_data->cs_gpiods &&
 	    chip->gpiod_cs)
-		gpio_free(desc_to_gpio(chip->gpiod_cs));
+		gpiod_free(chip->gpiod_cs);
 
 	kfree(chip);
 }
-- 
2.11.0

--
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

^ permalink raw reply related

* [GIT PULL] spi fixes for v4.15
From: Mark Brown @ 2017-12-20 17:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

The following changes since commit 1291a0d5049dbc06baaaf66a9ff3f53db493b19b:

  Linux 4.15-rc4 (2017-12-17 18:59:59 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-fix-v4.15-rc4

for you to fetch changes up to 4d029763724b6365c83c98f86f0958882efa8796:

  Merge remote-tracking branches 'spi/fix/armada', 'spi/fix/atmel', 'spi/fix/doc', 'spi/fix/imx', 'spi/fix/rspi', 'spi/fix/sun4i' and 'spi/fix/xilinx' into spi-linus (2017-12-19 11:07:00 +0000)

Hopefully I've managed to appease GMail, we shall see.

----------------------------------------------------------------
spi: Fixes for v4.15-rc4

A bunch of really small fixes here, all driver specific and mostly in
error handling and remove paths.  The most important fixes are for the
a3700 clock configuration and a fix for a nasty stall which could
potentially cause data corruption with the xilinx driver.

----------------------------------------------------------------
Geert Uytterhoeven (2):
      spi: Fix double "when"
      spi: rspi: Do not set SPCR_SPE in qspi_set_config_register()

Mark Brown (1):
      Merge remote-tracking branches 'spi/fix/armada', 'spi/fix/atmel', 'spi/fix/doc', 'spi/fix/imx', 'spi/fix/rspi', 'spi/fix/sun4i' and 'spi/fix/xilinx' into spi-linus

Maxime Chevallier (1):
      spi: a3700: Fix clk prescaling for coefficient over 15

Radu Pirea (1):
      spi: atmel: fixed spin_lock usage inside atmel_spi_remove

Ricardo Ribalda Delgado (1):
      spi: xilinx: Detect stall with Unknown commands

Takuo Koguchi (1):
      spi: sun4i: disable clocks in the remove function

Trent Piepho (1):
      spi: imx: Update device tree binding documentation

 Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt | 18 ++++++++++++------
 drivers/spi/spi-armada-3700.c                          |  8 ++++++++
 drivers/spi/spi-atmel.c                                |  2 +-
 drivers/spi/spi-rspi.c                                 |  4 ++--
 drivers/spi/spi-sun4i.c                                |  2 +-
 drivers/spi/spi-xilinx.c                               | 11 +++++++++++
 include/linux/spi/spi.h                                |  2 +-
 7 files changed, 36 insertions(+), 11 deletions(-)

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

^ permalink raw reply

* [PATCH v3] spi: atmel: Implements transfers with bounce buffer
From: Radu Pirea @ 2017-12-19 15:17 UTC (permalink / raw)
  To: linux-spi-u79uwXL29TY76Z2rM5mHXA, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Radu Pirea

This patch enables SPI DMA transfers for Atmel SAM9 SoCs and implements a
bounce buffer for transfers which have vmalloc allocated buffers. Those
buffers are not cache coherent even if they have been transformed into sg
lists. UBIFS is affected by this cache coherency issue.

In this patch I also reverted "spi: atmel: fix corrupted data issue on SAM9
family SoCs"(7094576ccdc3acfe1e06a1e2ab547add375baf7f).

Signed-off-by: Radu Pirea <radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
---
Changes v3:
-rebased on top of spi: atmel: fixed spin_lock usage inside atmel_spi_remove
-rerun checkpatch.pl and fix errors

Changes v2:
Please ignore the previous version. I messed up with file names.

 drivers/spi/spi-atmel.c | 113 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 84 insertions(+), 29 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 6694709..4a11fc0 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -291,6 +291,10 @@ struct atmel_spi {
 	struct spi_transfer	*current_transfer;
 	int			current_remaining_bytes;
 	int			done_status;
+	dma_addr_t		dma_addr_rx_bbuf;
+	dma_addr_t		dma_addr_tx_bbuf;
+	void			*addr_rx_bbuf;
+	void			*addr_tx_bbuf;
 
 	struct completion	xfer_completion;
 
@@ -436,6 +440,11 @@ static void atmel_spi_unlock(struct atmel_spi *as) __releases(&as->lock)
 	spin_unlock_irqrestore(&as->lock, as->flags);
 }
 
+static inline bool atmel_spi_is_vmalloc_xfer(struct spi_transfer *xfer)
+{
+	return is_vmalloc_addr(xfer->tx_buf) || is_vmalloc_addr(xfer->rx_buf);
+}
+
 static inline bool atmel_spi_use_dma(struct atmel_spi *as,
 				struct spi_transfer *xfer)
 {
@@ -448,7 +457,12 @@ static bool atmel_spi_can_dma(struct spi_master *master,
 {
 	struct atmel_spi *as = spi_master_get_devdata(master);
 
-	return atmel_spi_use_dma(as, xfer);
+	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5))
+		return atmel_spi_use_dma(as, xfer) &&
+			!atmel_spi_is_vmalloc_xfer(xfer);
+	else
+		return atmel_spi_use_dma(as, xfer);
+
 }
 
 static int atmel_spi_dma_slave_config(struct atmel_spi *as,
@@ -594,6 +608,11 @@ static void dma_callback(void *data)
 	struct spi_master	*master = data;
 	struct atmel_spi	*as = spi_master_get_devdata(master);
 
+	if (is_vmalloc_addr(as->current_transfer->rx_buf) &&
+	    IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+		memcpy(as->current_transfer->rx_buf, as->addr_rx_bbuf,
+		       as->current_transfer->len);
+	}
 	complete(&as->xfer_completion);
 }
 
@@ -744,17 +763,41 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
 		goto err_exit;
 
 	/* Send both scatterlists */
-	rxdesc = dmaengine_prep_slave_sg(rxchan,
-					 xfer->rx_sg.sgl, xfer->rx_sg.nents,
-					 DMA_FROM_DEVICE,
-					 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (atmel_spi_is_vmalloc_xfer(xfer) &&
+	    IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+		rxdesc = dmaengine_prep_slave_single(rxchan,
+						     as->dma_addr_rx_bbuf,
+						     xfer->len,
+						     DMA_FROM_DEVICE,
+						     DMA_PREP_INTERRUPT |
+						     DMA_CTRL_ACK);
+	} else {
+		rxdesc = dmaengine_prep_slave_sg(rxchan,
+						 xfer->rx_sg.sgl,
+						 xfer->rx_sg.nents,
+						 DMA_FROM_DEVICE,
+						 DMA_PREP_INTERRUPT |
+						 DMA_CTRL_ACK);
+	}
 	if (!rxdesc)
 		goto err_dma;
 
-	txdesc = dmaengine_prep_slave_sg(txchan,
-					 xfer->tx_sg.sgl, xfer->tx_sg.nents,
-					 DMA_TO_DEVICE,
-					 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (atmel_spi_is_vmalloc_xfer(xfer) &&
+	    IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+		memcpy(as->addr_tx_bbuf, xfer->tx_buf, xfer->len);
+		txdesc = dmaengine_prep_slave_single(txchan,
+						     as->dma_addr_tx_bbuf,
+						     xfer->len, DMA_TO_DEVICE,
+						     DMA_PREP_INTERRUPT |
+						     DMA_CTRL_ACK);
+	} else {
+		txdesc = dmaengine_prep_slave_sg(txchan,
+						 xfer->tx_sg.sgl,
+						 xfer->tx_sg.nents,
+						 DMA_TO_DEVICE,
+						 DMA_PREP_INTERRUPT |
+						 DMA_CTRL_ACK);
+	}
 	if (!txdesc)
 		goto err_dma;
 
@@ -1426,27 +1469,7 @@ static void atmel_get_caps(struct atmel_spi *as)
 
 	as->caps.is_spi2 = version > 0x121;
 	as->caps.has_wdrbt = version >= 0x210;
-#ifdef CONFIG_SOC_SAM_V4_V5
-	/*
-	 * Atmel SoCs based on ARM9 (SAM9x) cores should not use spi_map_buf()
-	 * since this later function tries to map buffers with dma_map_sg()
-	 * even if they have not been allocated inside DMA-safe areas.
-	 * On SoCs based on Cortex A5 (SAMA5Dx), it works anyway because for
-	 * those ARM cores, the data cache follows the PIPT model.
-	 * Also the L2 cache controller of SAMA5D2 uses the PIPT model too.
-	 * In case of PIPT caches, there cannot be cache aliases.
-	 * However on ARM9 cores, the data cache follows the VIVT model, hence
-	 * the cache aliases issue can occur when buffers are allocated from
-	 * DMA-unsafe areas, by vmalloc() for instance, where cache coherency is
-	 * not taken into account or at least not handled completely (cache
-	 * lines of aliases are not invalidated).
-	 * This is not a theorical issue: it was reproduced when trying to mount
-	 * a UBI file-system on a at91sam9g35ek board.
-	 */
-	as->caps.has_dma_support = false;
-#else
 	as->caps.has_dma_support = version >= 0x212;
-#endif
 	as->caps.has_pdc_support = version < 0x212;
 }
 
@@ -1592,6 +1615,30 @@ static int atmel_spi_probe(struct platform_device *pdev)
 		as->use_pdc = true;
 	}
 
+	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+		as->addr_rx_bbuf = dma_alloc_coherent(&pdev->dev,
+						      SPI_MAX_DMA_XFER,
+						      &as->dma_addr_rx_bbuf,
+						      GFP_KERNEL | GFP_DMA);
+		if (!as->addr_rx_bbuf) {
+			as->use_dma = false;
+		} else {
+			as->addr_tx_bbuf = dma_alloc_coherent(&pdev->dev,
+					SPI_MAX_DMA_XFER,
+					&as->dma_addr_tx_bbuf,
+					GFP_KERNEL | GFP_DMA);
+			if (!as->addr_tx_bbuf) {
+				as->use_dma = false;
+				dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
+						  as->addr_rx_bbuf,
+						  as->dma_addr_rx_bbuf);
+			}
+		}
+		if (!as->use_dma)
+			dev_info(master->dev.parent,
+				 "  can not allocate dma coherent memory\n");
+	}
+
 	if (as->caps.has_dma_support && !as->use_dma)
 		dev_info(&pdev->dev, "Atmel SPI Controller using PIO only\n");
 
@@ -1664,6 +1711,14 @@ static int atmel_spi_remove(struct platform_device *pdev)
 	if (as->use_dma) {
 		atmel_spi_stop_dma(master);
 		atmel_spi_release_dma(master);
+		if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
+			dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
+					  as->addr_tx_bbuf,
+					  as->dma_addr_tx_bbuf);
+			dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
+					  as->addr_rx_bbuf,
+					  as->dma_addr_rx_bbuf);
+		}
 	}
 
 	spin_lock_irq(&as->lock);
-- 
2.7.4

--
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

^ permalink raw reply related

* Applied "spi: atmel: fixed spin_lock usage inside atmel_spi_remove" to the spi tree
From: Mark Brown @ 2017-12-19 11:01 UTC (permalink / raw)
  To: Radu Pirea
  Cc: Jia-Ju Bai, Mark Brown, linux-spi, broonie, nicolas.ferre,
	alexandre.belloni, Wenyou.Yang, baijiaju1990, linux-kernel,
	linux-arm-kernel, linux-spi
In-Reply-To: <1513352417-15275-1-git-send-email-radu.pirea@microchip.com>

The patch

   spi: atmel: fixed spin_lock usage inside atmel_spi_remove

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 66e900a3d225575c8b48b59ae1fe74bb6e5a65cc Mon Sep 17 00:00:00 2001
From: Radu Pirea <radu.pirea@microchip.com>
Date: Fri, 15 Dec 2017 17:40:17 +0200
Subject: [PATCH] spi: atmel: fixed spin_lock usage inside atmel_spi_remove

The only part of atmel_spi_remove which needs to be atomic is hardware
reset.

atmel_spi_stop_dma calls dma_terminate_all and this needs interrupts
enabled.
atmel_spi_release_dma calls dma_release_channel and dma_release_channel
locks a mutex inside of spin_lock.

So the call of these functions can't be inside a spin_lock.

Reported-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-atmel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f95da364c283..669470971023 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1661,12 +1661,12 @@ static int atmel_spi_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	/* reset the hardware and block queue progress */
-	spin_lock_irq(&as->lock);
 	if (as->use_dma) {
 		atmel_spi_stop_dma(master);
 		atmel_spi_release_dma(master);
 	}
 
+	spin_lock_irq(&as->lock);
 	spi_writel(as, CR, SPI_BIT(SWRST));
 	spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
 	spi_readl(as, SR);
-- 
2.15.1

^ permalink raw reply related

* Re: [PATCH v2] spi: atmel: Implements transfers with bounce buffer
From: Alexandre Belloni @ 2017-12-19 10:43 UTC (permalink / raw)
  To: Radu Pirea
  Cc: linux-spi, broonie, nicolas.ferre, Wenyou.Yang, linux-kernel,
	linux-arm-kernel
In-Reply-To: <1513093032-11570-1-git-send-email-radu.pirea@microchip.com>

On 12/12/2017 at 17:37:12 +0200, Radu Pirea wrote:
> This patch enables DMA transfers for Atmel SAM9 SoCs and implements a bounce
> buffer for transfers which have vmalloc allocated buffers. Those buffers are
> not cache coherent even if they have been transformed into sg lists. UBIFS
> is affected by this cache coherency issue.
> 
> In this patch I also reverted "spi: atmel: fix corrupted data issue on SAM9
> family SoCs"(7094576ccdc3acfe1e06a1e2ab547add375baf7f).
> 
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  Please ignore the previous version. I messed up with file names.
>  drivers/spi/spi-atmel.c | 113 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 84 insertions(+), 29 deletions(-)
> 

There are multiple checkpatch issues, can you fix them?

> +		if(!as->use_dma)

Especially that missing space.

> +			dev_info(master->dev.parent,
> +				"  can not allocate dma coherent memory\n");
> +	}
> +

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2] spi: atmel: Implements transfers with bounce buffer
From: Nicolas Ferre @ 2017-12-19 10:25 UTC (permalink / raw)
  To: Radu Pirea, broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1513093032-11570-1-git-send-email-radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

On 12/12/2017 at 16:37, Radu Pirea wrote:
> This patch enables DMA transfers for Atmel SAM9 SoCs and implements a bounce
> buffer for transfers which have vmalloc allocated buffers. Those buffers are
> not cache coherent even if they have been transformed into sg lists. UBIFS
> is affected by this cache coherency issue.
> 
> In this patch I also reverted "spi: atmel: fix corrupted data issue on SAM9
> family SoCs"(7094576ccdc3acfe1e06a1e2ab547add375baf7f).
> 
> 
> Signed-off-by: Radu Pirea <radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

Best regards,
  Nicolas

> ---
>  Please ignore the previous version. I messed up with file names.
>  drivers/spi/spi-atmel.c | 113 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 84 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index f95da36..198b4cd 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -291,6 +291,10 @@ struct atmel_spi {
>  	struct spi_transfer	*current_transfer;
>  	int			current_remaining_bytes;
>  	int			done_status;
> +	dma_addr_t		dma_addr_rx_bbuf;
> +	dma_addr_t		dma_addr_tx_bbuf;
> +	void			*addr_rx_bbuf;
> +	void			*addr_tx_bbuf;
>  
>  	struct completion	xfer_completion;
>  
> @@ -436,6 +440,11 @@ static void atmel_spi_unlock(struct atmel_spi *as) __releases(&as->lock)
>  	spin_unlock_irqrestore(&as->lock, as->flags);
>  }
>  
> +static inline bool atmel_spi_is_vmalloc_xfer(struct spi_transfer *xfer)
> +{
> +	return is_vmalloc_addr(xfer->tx_buf) || is_vmalloc_addr(xfer->rx_buf);
> +}
> +
>  static inline bool atmel_spi_use_dma(struct atmel_spi *as,
>  				struct spi_transfer *xfer)
>  {
> @@ -448,7 +457,12 @@ static bool atmel_spi_can_dma(struct spi_master *master,
>  {
>  	struct atmel_spi *as = spi_master_get_devdata(master);
>  
> -	return atmel_spi_use_dma(as, xfer);
> +	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5))
> +		return atmel_spi_use_dma(as, xfer) &&
> +			!atmel_spi_is_vmalloc_xfer(xfer);
> +	else
> +		return atmel_spi_use_dma(as, xfer);
> +
>  }
>  
>  static int atmel_spi_dma_slave_config(struct atmel_spi *as,
> @@ -594,6 +608,11 @@ static void dma_callback(void *data)
>  	struct spi_master	*master = data;
>  	struct atmel_spi	*as = spi_master_get_devdata(master);
>  
> +	if (is_vmalloc_addr(as->current_transfer->rx_buf) &&
> +	    IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +		memcpy(as->current_transfer->rx_buf, as->addr_rx_bbuf,
> +		       as->current_transfer->len);
> +	}
>  	complete(&as->xfer_completion);
>  }
>  
> @@ -744,17 +763,41 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
>  		goto err_exit;
>  
>  	/* Send both scatterlists */
> -	rxdesc = dmaengine_prep_slave_sg(rxchan,
> -					 xfer->rx_sg.sgl, xfer->rx_sg.nents,
> -					 DMA_FROM_DEVICE,
> -					 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (atmel_spi_is_vmalloc_xfer(xfer) &&
> +	    IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +		rxdesc = dmaengine_prep_slave_single(rxchan,
> +						     as->dma_addr_rx_bbuf,
> +						     xfer->len,
> +						     DMA_FROM_DEVICE,
> +						     DMA_PREP_INTERRUPT |
> +						     DMA_CTRL_ACK);
> +	} else {
> +		rxdesc = dmaengine_prep_slave_sg(rxchan,
> +						 xfer->rx_sg.sgl,
> +						 xfer->rx_sg.nents,
> +						 DMA_FROM_DEVICE,
> +						 DMA_PREP_INTERRUPT |
> +						 DMA_CTRL_ACK);
> +	}
>  	if (!rxdesc)
>  		goto err_dma;
>  
> -	txdesc = dmaengine_prep_slave_sg(txchan,
> -					 xfer->tx_sg.sgl, xfer->tx_sg.nents,
> -					 DMA_TO_DEVICE,
> -					 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +	if (atmel_spi_is_vmalloc_xfer(xfer) &&
> +	    IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +		memcpy(as->addr_tx_bbuf, xfer->tx_buf, xfer->len);
> +		txdesc = dmaengine_prep_slave_single(txchan,
> +						     as->dma_addr_tx_bbuf,
> +						     xfer->len, DMA_TO_DEVICE,
> +						     DMA_PREP_INTERRUPT |
> +						     DMA_CTRL_ACK);
> +	} else {
> +		txdesc = dmaengine_prep_slave_sg(txchan,
> +						 xfer->tx_sg.sgl,
> +						 xfer->tx_sg.nents,
> +						 DMA_TO_DEVICE,
> +						 DMA_PREP_INTERRUPT |
> +						 DMA_CTRL_ACK);
> +	}
>  	if (!txdesc)
>  		goto err_dma;
>  
> @@ -1426,27 +1469,7 @@ static void atmel_get_caps(struct atmel_spi *as)
>  
>  	as->caps.is_spi2 = version > 0x121;
>  	as->caps.has_wdrbt = version >= 0x210;
> -#ifdef CONFIG_SOC_SAM_V4_V5
> -	/*
> -	 * Atmel SoCs based on ARM9 (SAM9x) cores should not use spi_map_buf()
> -	 * since this later function tries to map buffers with dma_map_sg()
> -	 * even if they have not been allocated inside DMA-safe areas.
> -	 * On SoCs based on Cortex A5 (SAMA5Dx), it works anyway because for
> -	 * those ARM cores, the data cache follows the PIPT model.
> -	 * Also the L2 cache controller of SAMA5D2 uses the PIPT model too.
> -	 * In case of PIPT caches, there cannot be cache aliases.
> -	 * However on ARM9 cores, the data cache follows the VIVT model, hence
> -	 * the cache aliases issue can occur when buffers are allocated from
> -	 * DMA-unsafe areas, by vmalloc() for instance, where cache coherency is
> -	 * not taken into account or at least not handled completely (cache
> -	 * lines of aliases are not invalidated).
> -	 * This is not a theorical issue: it was reproduced when trying to mount
> -	 * a UBI file-system on a at91sam9g35ek board.
> -	 */
> -	as->caps.has_dma_support = false;
> -#else
>  	as->caps.has_dma_support = version >= 0x212;
> -#endif
>  	as->caps.has_pdc_support = version < 0x212;
>  }
>  
> @@ -1592,6 +1615,30 @@ static int atmel_spi_probe(struct platform_device *pdev)
>  		as->use_pdc = true;
>  	}
>  
> +	if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +		as->addr_rx_bbuf = dma_alloc_coherent(&pdev->dev,
> +						      SPI_MAX_DMA_XFER,
> +						      &as->dma_addr_rx_bbuf,
> +						      GFP_KERNEL | GFP_DMA);
> +		if (!as->addr_rx_bbuf) {
> +			as->use_dma = false;
> +		} else {
> +			as->addr_tx_bbuf = dma_alloc_coherent(&pdev->dev,
> +					SPI_MAX_DMA_XFER,
> +					&as->dma_addr_tx_bbuf,
> +					GFP_KERNEL | GFP_DMA);
> +			if (!as->addr_tx_bbuf) {
> +				as->use_dma = false;
> +				dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
> +						  as->addr_rx_bbuf,
> +						  as->dma_addr_rx_bbuf);
> +			}
> +		}
> +		if(!as->use_dma)
> +			dev_info(master->dev.parent,
> +				"  can not allocate dma coherent memory\n");
> +	}
> +
>  	if (as->caps.has_dma_support && !as->use_dma)
>  		dev_info(&pdev->dev, "Atmel SPI Controller using PIO only\n");
>  
> @@ -1665,6 +1712,14 @@ static int atmel_spi_remove(struct platform_device *pdev)
>  	if (as->use_dma) {
>  		atmel_spi_stop_dma(master);
>  		atmel_spi_release_dma(master);
> +		if (IS_ENABLED(CONFIG_SOC_SAM_V4_V5)) {
> +			dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
> +					  as->addr_tx_bbuf,
> +					  as->dma_addr_tx_bbuf);
> +			dma_free_coherent(&pdev->dev, SPI_MAX_DMA_XFER,
> +					  as->addr_rx_bbuf,
> +					  as->dma_addr_rx_bbuf);
> +		}
>  	}
>  
>  	spi_writel(as, CR, SPI_BIT(SWRST));
> 


-- 
Nicolas Ferre
--
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

^ permalink raw reply

* Re: [PATCH] spi: atmel: fixed spin_lock usage inside atmel_spi_remove
From: Nicolas Ferre @ 2017-12-19 10:23 UTC (permalink / raw)
  To: Radu Pirea, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA,
	baijiaju1990-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1513352417-15275-1-git-send-email-radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

On 15/12/2017 at 16:40, Radu Pirea wrote:
> The only part of atmel_spi_remove which needs to be atomic is hardware
> reset.
> 
> atmel_spi_stop_dma calls dma_terminate_all and this needs interrupts
> enabled.
> atmel_spi_release_dma calls dma_release_channel and dma_release_channel
> locks a mutex inside of spin_lock.
> 
> So the call of these functions can't be inside a spin_lock.
> 
> Reported-by: Jia-Ju Bai <baijiaju1990-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Radu Pirea <radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

Thanks Radu. Regards,
  Nicolas

> ---
>  drivers/spi/spi-atmel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index f95da36..6694709 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -1661,12 +1661,12 @@ static int atmel_spi_remove(struct platform_device *pdev)
>  	pm_runtime_get_sync(&pdev->dev);
>  
>  	/* reset the hardware and block queue progress */
> -	spin_lock_irq(&as->lock);
>  	if (as->use_dma) {
>  		atmel_spi_stop_dma(master);
>  		atmel_spi_release_dma(master);
>  	}
>  
> +	spin_lock_irq(&as->lock);
>  	spi_writel(as, CR, SPI_BIT(SWRST));
>  	spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
>  	spi_readl(as, SR);
> 


-- 
Nicolas Ferre
--
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

^ permalink raw reply

* [PATCH] spi: atmel: fixed spin_lock usage inside atmel_spi_remove
From: Radu Pirea @ 2017-12-15 15:40 UTC (permalink / raw)
  To: linux-spi, broonie, nicolas.ferre, alexandre.belloni, Wenyou.Yang,
	baijiaju1990
  Cc: linux-kernel, linux-arm-kernel, Radu Pirea

The only part of atmel_spi_remove which needs to be atomic is hardware
reset.

atmel_spi_stop_dma calls dma_terminate_all and this needs interrupts
enabled.
atmel_spi_release_dma calls dma_release_channel and dma_release_channel
locks a mutex inside of spin_lock.

So the call of these functions can't be inside a spin_lock.

Reported-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/spi/spi-atmel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f95da36..6694709 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1661,12 +1661,12 @@ static int atmel_spi_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	/* reset the hardware and block queue progress */
-	spin_lock_irq(&as->lock);
 	if (as->use_dma) {
 		atmel_spi_stop_dma(master);
 		atmel_spi_release_dma(master);
 	}
 
+	spin_lock_irq(&as->lock);
 	spi_writel(as, CR, SPI_BIT(SWRST));
 	spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
 	spi_readl(as, SR);
-- 
2.7.4

^ permalink raw reply related

* Applied "spi: sh-msiof: Avoid writing to registers from spi_master.setup()" to the spi tree
From: Mark Brown @ 2017-12-14 11:50 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Mark Brown, Rob Herring, Mark Rutland, linux-spi,
	devicetree, linux-renesas-soc, linux-spi
In-Reply-To: <1513191913-10612-2-git-send-email-geert+renesas@glider.be>

The patch

   spi: sh-msiof: Avoid writing to registers from spi_master.setup()

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 7ff0b53c4051145d1cf992d2f60987e6447eed4f Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 13 Dec 2017 20:05:10 +0100
Subject: [PATCH] spi: sh-msiof: Avoid writing to registers from
 spi_master.setup()

The spi_master.setup() callback must not change configuration registers,
as that could corrupt I/O that is in progress for other SPI slaves.

The only exception is the configuration of the native chip select
polarity in SPI master mode, as a wrong chip select polarity will cause
havoc during all future transfers to any other SPI slave.

Hence stop writing to registers in sh_msiof_spi_setup(), unless it is
the first call for a controller using a native chip select, or unless
native chip select polarity has changed (note that you'll loose anyway
if I/O is in progress).  Even then, only do what is strictly necessary,
instead of calling sh_msiof_spi_set_pin_regs().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-sh-msiof.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 81a9144f5442..2704abb11ea4 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -55,6 +55,8 @@ struct sh_msiof_spi_priv {
 	void *rx_dma_page;
 	dma_addr_t tx_dma_addr;
 	dma_addr_t rx_dma_addr;
+	bool native_cs_inited;
+	bool native_cs_high;
 	bool slave_aborted;
 };
 
@@ -528,8 +530,7 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 {
 	struct device_node	*np = spi->master->dev.of_node;
 	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
-
-	pm_runtime_get_sync(&p->pdev->dev);
+	u32 clr, set, tmp;
 
 	if (!np) {
 		/*
@@ -539,19 +540,31 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 		spi->cs_gpio = (uintptr_t)spi->controller_data;
 	}
 
-	/* Configure pins before deasserting CS */
-	sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
-				  !!(spi->mode & SPI_CPHA),
-				  !!(spi->mode & SPI_3WIRE),
-				  !!(spi->mode & SPI_LSB_FIRST),
-				  !!(spi->mode & SPI_CS_HIGH));
-
-	if (spi->cs_gpio >= 0)
+	if (spi->cs_gpio >= 0) {
 		gpio_set_value(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
+		return 0;
+	}
 
+	if (spi_controller_is_slave(p->master))
+		return 0;
 
-	pm_runtime_put(&p->pdev->dev);
+	if (p->native_cs_inited &&
+	    (p->native_cs_high == !!(spi->mode & SPI_CS_HIGH)))
+		return 0;
 
+	/* Configure native chip select mode/polarity early */
+	clr = MDR1_SYNCMD_MASK;
+	set = MDR1_TRMD | TMDR1_PCON | MDR1_SYNCMD_SPI;
+	if (spi->mode & SPI_CS_HIGH)
+		clr |= BIT(MDR1_SYNCAC_SHIFT);
+	else
+		set |= BIT(MDR1_SYNCAC_SHIFT);
+	pm_runtime_get_sync(&p->pdev->dev);
+	tmp = sh_msiof_read(p, TMDR1) & ~clr;
+	sh_msiof_write(p, TMDR1, tmp | set);
+	pm_runtime_put(&p->pdev->dev);
+	p->native_cs_high = spi->mode & SPI_CS_HIGH;
+	p->native_cs_inited = true;
 	return 0;
 }
 
-- 
2.15.1

^ permalink raw reply related

* Applied "spi: sh-msiof: Extend support to 3 native chip selects" to the spi tree
From: Mark Brown @ 2017-12-14 11:50 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Mark Brown, Rob Herring, Mark Rutland, linux-spi,
	devicetree, linux-renesas-soc, linux-spi
In-Reply-To: <1513191913-10612-3-git-send-email-geert+renesas@glider.be>

The patch

   spi: sh-msiof: Extend support to 3 native chip selects

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9cce882bedd2768dc251b73f2ad86a9bfcfd9fc7 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 13 Dec 2017 20:05:11 +0100
Subject: [PATCH] spi: sh-msiof: Extend support to 3 native chip selects

Currently only the MSIOF_SYNC signal can be used as a native chip
select.  Extend support to up to 3 native chipselects using the
MSIOF_SS1 and MSIOF_SS2 signals.

Inspired by a patch in the BSP by Hiromitsu Yamasaki.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt |  6 +++++-
 drivers/spi/spi-sh-msiof.c                         | 18 +++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index bdd83959019c..bc8c16a6cfc8 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -36,7 +36,11 @@ Required properties:
 
 Optional properties:
 - clocks               : Must contain a reference to the functional clock.
-- num-cs               : Total number of chip-selects (default is 1)
+- num-cs               : Total number of chip selects (default is 1).
+			 Up to 3 native chip selects are supported:
+			   0: MSIOF_SYNC
+			   1: MSIOF_SS1
+			   2: MSIOF_SS2
 - dmas                 : Must contain a list of two references to DMA
 			 specifiers, one for transmission, and one for
 			 reception.
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 2704abb11ea4..9bdc292aa050 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -60,6 +60,8 @@ struct sh_msiof_spi_priv {
 	bool slave_aborted;
 };
 
+#define MAX_SS	3	/* Maximum number of native chip selects */
+
 #define TMDR1	0x00	/* Transmit Mode Register 1 */
 #define TMDR2	0x04	/* Transmit Mode Register 2 */
 #define TMDR3	0x08	/* Transmit Mode Register 3 */
@@ -93,6 +95,8 @@ struct sh_msiof_spi_priv {
 #define MDR1_XXSTP	 0x00000001 /* Transmission/Reception Stop on FIFO */
 /* TMDR1 */
 #define TMDR1_PCON	 0x40000000 /* Transfer Signal Connection */
+#define TMDR1_SYNCCH_MASK 0xc000000 /* Synchronization Signal Channel Select */
+#define TMDR1_SYNCCH_SHIFT	 26 /* 0=MSIOF_SYNC, 1=MSIOF_SS1, 2=MSIOF_SS2 */
 
 /* TMDR2 and RMDR2 */
 #define MDR2_BITLEN1(i)	(((i) - 1) << 24) /* Data Size (8-32 bits) */
@@ -326,7 +330,7 @@ static u32 sh_msiof_spi_get_dtdl_and_syncdl(struct sh_msiof_spi_priv *p)
 	return val;
 }
 
-static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
+static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, u32 ss,
 				      u32 cpol, u32 cpha,
 				      u32 tx_hi_z, u32 lsb_first, u32 cs_high)
 {
@@ -344,10 +348,13 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
 	tmp |= !cs_high << MDR1_SYNCAC_SHIFT;
 	tmp |= lsb_first << MDR1_BITLSB_SHIFT;
 	tmp |= sh_msiof_spi_get_dtdl_and_syncdl(p);
-	if (spi_controller_is_slave(p->master))
+	if (spi_controller_is_slave(p->master)) {
 		sh_msiof_write(p, TMDR1, tmp | TMDR1_PCON);
-	else
-		sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON);
+	} else {
+		sh_msiof_write(p, TMDR1,
+			       tmp | MDR1_TRMD | TMDR1_PCON |
+			       (ss < MAX_SS ? ss : 0) << TMDR1_SYNCCH_SHIFT);
+	}
 	if (p->master->flags & SPI_MASTER_MUST_TX) {
 		/* These bits are reserved if RX needs TX */
 		tmp &= ~0x0000ffff;
@@ -575,7 +582,8 @@ static int sh_msiof_prepare_message(struct spi_master *master,
 	const struct spi_device *spi = msg->spi;
 
 	/* Configure pins before asserting CS */
-	sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
+	sh_msiof_spi_set_pin_regs(p, spi->chip_select,
+				  !!(spi->mode & SPI_CPOL),
 				  !!(spi->mode & SPI_CPHA),
 				  !!(spi->mode & SPI_3WIRE),
 				  !!(spi->mode & SPI_LSB_FIRST),
-- 
2.15.1

^ permalink raw reply related

* Applied "spi: sh-msiof: Implement cs-gpios configuration" to the spi tree
From: Mark Brown @ 2017-12-14 11:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Mark Brown, Rob Herring, Mark Rutland, linux-spi,
	devicetree, linux-renesas-soc, linux-spi
In-Reply-To: <1513191913-10612-4-git-send-email-geert+renesas@glider.be>

The patch

   spi: sh-msiof: Implement cs-gpios configuration

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From b8761434bdec32fa46a644c26a12d16a9b0f58d8 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 13 Dec 2017 20:05:12 +0100
Subject: [PATCH] spi: sh-msiof: Implement cs-gpios configuration

The current support for GPIO chip selects assumes the GPIOs have been
configured by platform code or the boot loader.  This includes pinmux
setup and GPIO direction.  Hence it does not work as expected when just
described in DT using the "cs-gpios" property.

Fix this by:
  1. using devm_gpiod_get_index() to request the GPIO, and thus
     configure pinmux, if needed,
  2. configuring the GPIO direction is the spi_master.setup() callback.

Use gpio_is_valid() instead of a check on positive numbers.

Note that when using GPIO chip selects, at least one native chip select
must be left unused, as that native chip select will be driven anyway,
and (global) native chip select polarity must be taken into account.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-sh-msiof.c | 66 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 9bdc292aa050..8aa5c7b910d9 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -19,6 +19,7 @@
 #include <linux/dmaengine.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -55,6 +56,7 @@ struct sh_msiof_spi_priv {
 	void *rx_dma_page;
 	dma_addr_t tx_dma_addr;
 	dma_addr_t rx_dma_addr;
+	unsigned short unused_ss;
 	bool native_cs_inited;
 	bool native_cs_high;
 	bool slave_aborted;
@@ -547,8 +549,8 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 		spi->cs_gpio = (uintptr_t)spi->controller_data;
 	}
 
-	if (spi->cs_gpio >= 0) {
-		gpio_set_value(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
+	if (gpio_is_valid(spi->cs_gpio)) {
+		gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
 		return 0;
 	}
 
@@ -580,14 +582,20 @@ static int sh_msiof_prepare_message(struct spi_master *master,
 {
 	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
 	const struct spi_device *spi = msg->spi;
+	u32 ss, cs_high;
 
 	/* Configure pins before asserting CS */
-	sh_msiof_spi_set_pin_regs(p, spi->chip_select,
-				  !!(spi->mode & SPI_CPOL),
+	if (gpio_is_valid(spi->cs_gpio)) {
+		ss = p->unused_ss;
+		cs_high = p->native_cs_high;
+	} else {
+		ss = spi->chip_select;
+		cs_high = !!(spi->mode & SPI_CS_HIGH);
+	}
+	sh_msiof_spi_set_pin_regs(p, ss, !!(spi->mode & SPI_CPOL),
 				  !!(spi->mode & SPI_CPHA),
 				  !!(spi->mode & SPI_3WIRE),
-				  !!(spi->mode & SPI_LSB_FIRST),
-				  !!(spi->mode & SPI_CS_HIGH));
+				  !!(spi->mode & SPI_LSB_FIRST), cs_high);
 	return 0;
 }
 
@@ -1091,6 +1099,45 @@ static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
 }
 #endif
 
+static int sh_msiof_get_cs_gpios(struct sh_msiof_spi_priv *p)
+{
+	struct device *dev = &p->pdev->dev;
+	unsigned int used_ss_mask = 0;
+	unsigned int cs_gpios = 0;
+	unsigned int num_cs, i;
+	int ret;
+
+	ret = gpiod_count(dev, "cs");
+	if (ret <= 0)
+		return 0;
+
+	num_cs = max_t(unsigned int, ret, p->master->num_chipselect);
+	for (i = 0; i < num_cs; i++) {
+		struct gpio_desc *gpiod;
+
+		gpiod = devm_gpiod_get_index(dev, "cs", i, GPIOD_ASIS);
+		if (!IS_ERR(gpiod)) {
+			cs_gpios++;
+			continue;
+		}
+
+		if (PTR_ERR(gpiod) != -ENOENT)
+			return PTR_ERR(gpiod);
+
+		if (i >= MAX_SS) {
+			dev_err(dev, "Invalid native chip select %d\n", i);
+			return -EINVAL;
+		}
+		used_ss_mask |= BIT(i);
+	}
+	p->unused_ss = ffz(used_ss_mask);
+	if (cs_gpios && p->unused_ss >= MAX_SS) {
+		dev_err(dev, "No unused native chip select available\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static struct dma_chan *sh_msiof_request_dma_chan(struct device *dev,
 	enum dma_transfer_direction dir, unsigned int id, dma_addr_t port_addr)
 {
@@ -1304,13 +1351,18 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	if (p->info->rx_fifo_override)
 		p->rx_fifo_size = p->info->rx_fifo_override;
 
+	/* Setup GPIO chip selects */
+	master->num_chipselect = p->info->num_chipselect;
+	ret = sh_msiof_get_cs_gpios(p);
+	if (ret)
+		goto err1;
+
 	/* init master code */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
 	master->flags = chipdata->master_flags;
 	master->bus_num = pdev->id;
 	master->dev.of_node = pdev->dev.of_node;
-	master->num_chipselect = p->info->num_chipselect;
 	master->setup = sh_msiof_spi_setup;
 	master->prepare_message = sh_msiof_prepare_message;
 	master->slave_abort = sh_msiof_slave_abort;
-- 
2.15.1

^ permalink raw reply related

* Applied "spi: sh-msiof: Document hardware limitations related to chip selects" to the spi tree
From: Mark Brown @ 2017-12-14 11:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Mark Brown, Rob Herring, Mark Rutland, linux-spi,
	devicetree, linux-renesas-soc, linux-spi
In-Reply-To: <1513191913-10612-5-git-send-email-geert+renesas@glider.be>

The patch

   spi: sh-msiof: Document hardware limitations related to chip selects

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c99182f73cce7926c623b5c1c0ff0b7954ac8d81 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 13 Dec 2017 20:05:13 +0100
Subject: [PATCH] spi: sh-msiof: Document hardware limitations related to chip
 selects

Guide users to maintain the proper balance between native and GPIO chip
selects.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index bc8c16a6cfc8..80710f0f0448 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -41,6 +41,16 @@ Optional properties:
 			   0: MSIOF_SYNC
 			   1: MSIOF_SS1
 			   2: MSIOF_SS2
+			 Hardware limitations related to chip selects:
+			   - Native chip selects are always deasserted in
+			     between transfers that are part of the same
+			     message.  Use cs-gpios to work around this.
+			   - All slaves using native chip selects must use the
+			     same spi-cs-high configuration.  Use cs-gpios to
+			     work around this.
+			   - When using GPIO chip selects, at least one native
+			     chip select must be left unused, as it will be
+			     driven anyway.
 - dmas                 : Must contain a list of two references to DMA
 			 specifiers, one for transmission, and one for
 			 reception.
-- 
2.15.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox