gmane.linux.hardware.blackfin.kernel.devel

Re: [PATCH v2] spi: add spi controller master driver for Blackfin 6xx processor

Mark Brown — 2013-06-19T18:39:28

A few nits here but mostly looking good.


Just set bits_per_word_mask in the master struct and let the core do
this for you.


Braces on both sides of the if.


devm_ioremap_resource() will display an error message for you so you
don't need to bother, though in any case the return code should
generally be displayed.


Use SET_SYSTEM_SLEEP_PM_OPS().


Should really use module_platform_driver() on modern systems, the
deferred probe infrastructure should remove the need for faffing around
with init order.


Why not call the driver something like bfin_spi3 then?
_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b< at >public.gmane.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

Re: [PATCH] spi: add spi controller master driver for Blackfin 6xx processor

Mark Brown — 2013-06-06T09:59:57




Yes but when I grep for "do not set bits that the SPI framework manages"
after I see it in my kernel output I won't be able to find it because
you split it over multiple lines in the source file.




That doesn't address my concern - what happens if the hardware messed up
and didn't set either bit?



OK, document this then - otherwise someone will just come along and
delete the code since it's not obviously sensible.
_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b< at >public.gmane.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

Re: [PATCH] spi: add spi controller master driver for Blackfin 6xx processor

Mark Brown — 2013-06-03T14:28:09


This looks icky due to the casts.  Why not just store the state in the
driver data for the device, it's not possible to have multiple messages
active at once anyway?


Just from a style poin of view the global variable here isn't great,
even if it's always global for the system currently it'd be better to
have it in the data structure.


Looking at this I can't help but think there must be more common code we
can factor out but that's a separate thing to adding a new driver.


This isn't a triumph of legibility and really ought to be factored out
into the core as the same will be true for all SPI transfers.



The pump function is a pretty big function and this if in the middle of
it makes the flow a bit odd.  It would probably be better to split out
DMA and PIO transfer functions and then make this just call whichever is
appropriate.


It seems like we'd want to deassert /CS even on error but we didn't do
that...


Write this as a switch statement.


Don't split error messages over lines.


Why is all thi

[PATCH] spi: add spi controller master driverfor Blackfin 6xx processor

Scott Jiang — 2013-06-03T21:43:34

New spi controller is integrated into Blackfin 6xx processor.
Comparing to bf5xx spi controller, we support 32 bits word size
and independent receive and transmit DMA channels now.
Also mode 0 and 2 (CPHA = 0) can get fully supported
becasue cs line may be controlled by the software.

Signed-off-by: Scott Jiang <scott.jiang.linux-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>
---
 drivers/spi/Kconfig       |    8 +-
 drivers/spi/Makefile      |    1 +
 drivers/spi/spi-bfin6xx.c | 1026 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1034 insertions(+), 1 deletions(-)
 create mode 100644 drivers/spi/spi-bfin6xx.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 864d87f..d64bbeb 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
< at >< at > -87,10 +87,16 < at >< at > config SPI_BCM2835
 
 config SPI_BFIN5XX
 tristate "SPI controller driver for ADI Blackfin5xx"
-depends on BLACKFIN
+depends on BLACKFIN && !BF60x
 help
   This is the SPI controller master driver for Blackfin 5xx processor.
 
+con

Re: [PATCH -next] blackfin: bf533-stamp:Remove bogus "||"

Lars-Peter Clausen — 2013-05-31T11:37:41

Hm, right I missed that one, thanks.

Acked-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>

Mark can you queue it up in your topic/blackfin branch?

[PATCH 2/2] dma: Set PAUSE_RESUME capabilityfor drivers which support it

Lars-Peter Clausen — 2013-05-29T11:42:23

Set the newly introduced PAUSE_RESUME capability for driver which support
pausing/resuming the DMA stream.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 drivers/dma/amba-pl08x.c    | 1 +
 drivers/dma/at_hdmac.c      | 1 +
 drivers/dma/coh901318.c     | 1 +
 drivers/dma/dw_dmac.c       | 1 +
 drivers/dma/ipu/ipu_idmac.c | 1 +
 drivers/dma/mmp_tdma.c      | 1 +
 drivers/dma/mxs-dma.c       | 1 +
 drivers/dma/omap-dma.c      | 1 +
 drivers/dma/sa11x0-dma.c    | 1 +
 drivers/dma/sirf-dma.c      | 1 +
 drivers/dma/ste_dma40.c     | 1 +
 11 files changed, 11 insertions(+)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 8bad254..fa25199 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
< at >< at > -1873,6 +1873,7 < at >< at > static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
 
 /* Initialize slave engine */
 dma_cap_set(DMA_SLAVE, pl08x->slave.cap_mask);
+dma_cap_set(DMA_PAUSE_RESUME, pl08x->slave.cap_mask);
 pl08x->slave.

[PATCH 07/10] ASoC: blackfin: Switchbf5xx-ad193x from bf5xx-tdm to bf5xx-i2s

Lars-Peter Clausen — 2013-05-28T17:22:15

The bf5xx-i2s driver now has support for TDM mode and the bf5xx-tdm driver is
going to be removed soon, so switch the driver over to bf5xx-i2s.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 sound/soc/blackfin/Kconfig        | 18 +++++++++---------
 sound/soc/blackfin/bf5xx-ad193x.c | 39 ++++++++++-----------------------------
 2 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/sound/soc/blackfin/Kconfig b/sound/soc/blackfin/Kconfig
index 16b88f5..906c349 100644
--- a/sound/soc/blackfin/Kconfig
+++ b/sound/soc/blackfin/Kconfig
< at >< at > -56,6 +56,15 < at >< at > config SND_SOC_BFIN_EVAL_ADAV80X
   Note: This driver assumes that the ADAV80X digital record and playback
   interfaces are connected to the first SPORT port on the BF5XX board.
 
+config SND_BF5XX_SOC_AD193X
+tristate "SoC AD193X Audio support for Blackfin"
+depends on SND_BF5XX_I2S
+select SND_BF5XX_SOC_I2S
+select SND_SOC_AD193X
+help
+  Say Y if you want to add support for AD193X codec on Blackfin.
+

[PATCH 10/10] blackfin: Remove references tothe bf5x_tdm driver

Lars-Peter Clausen — 2013-05-28T17:22:18

The bf5x_tdm driver has been removed. Remove all references to it from board
code.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 arch/blackfin/mach-bf527/boards/ad7160eval.c | 12 -----------
 arch/blackfin/mach-bf527/boards/ezkit.c      | 30 +--------------------------
 arch/blackfin/mach-bf533/boards/ezkit.c      | 12 -----------
 arch/blackfin/mach-bf533/boards/stamp.c      | 31 ----------------------------
 arch/blackfin/mach-bf537/boards/stamp.c      | 28 -------------------------
 arch/blackfin/mach-bf548/boards/ezkit.c      | 28 +------------------------
 arch/blackfin/mach-bf561/boards/ezkit.c      | 12 -----------
 7 files changed, 2 insertions(+), 151 deletions(-)

diff --git a/arch/blackfin/mach-bf527/boards/ad7160eval.c b/arch/blackfin/mach-bf527/boards/ad7160eval.c
index d58f50e..1e7be62 100644
--- a/arch/blackfin/mach-bf527/boards/ad7160eval.c
+++ b/arch/blackfin/mach-bf527/boards/ad7160eval.c
< at >< at > -283,14 +283,6 < at >< at > static struct platform_device bfin_i2s =

[PATCH 09/10] ASoC: blackfin: Remove bf5xx-tdmdriver

Lars-Peter Clausen — 2013-05-28T17:22:17

Now that the bf5xx-i2s driver supports TDM mode and all users of the bf5xx-tdm
driver have been switch over to using the bf5xx-i2s driver there is no need to
keep the b5fxx-tdm driver around.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 sound/soc/blackfin/Kconfig         |  13 --
 sound/soc/blackfin/Makefile        |   4 -
 sound/soc/blackfin/bf5xx-tdm-pcm.c | 343 -------------------------------------
 sound/soc/blackfin/bf5xx-tdm.c     | 328 -----------------------------------
 sound/soc/blackfin/bf5xx-tdm.h     |  23 ---
 5 files changed, 711 deletions(-)
 delete mode 100644 sound/soc/blackfin/bf5xx-tdm-pcm.c
 delete mode 100644 sound/soc/blackfin/bf5xx-tdm.c
 delete mode 100644 sound/soc/blackfin/bf5xx-tdm.h

diff --git a/sound/soc/blackfin/Kconfig b/sound/soc/blackfin/Kconfig
index 4a67865..54f74f8 100644
--- a/sound/soc/blackfin/Kconfig
+++ b/sound/soc/blackfin/Kconfig
< at >< at > -89,16 +89,6 < at >< at > config SND_BFIN_AD73311_SE
   Enter the GPIO used to control AD73311's SE

[PATCH 08/10] ASoC: blackfin: Switchbf5xx-ad1836 from bf5xx-tdm to bf5xx-i2s

Lars-Peter Clausen — 2013-05-28T17:22:16

The bf5xx-i2s driver now has support for TDM mode and the bf5xx-tdm driver is
going to be removed soon, so switch the driver over to bf5xx-i2s.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 arch/blackfin/mach-bf527/boards/ezkit.c |  2 +-
 arch/blackfin/mach-bf533/boards/stamp.c |  2 +-
 arch/blackfin/mach-bf537/boards/stamp.c |  2 +-
 arch/blackfin/mach-bf561/boards/ezkit.c |  2 +-
 arch/blackfin/mach-bf609/boards/ezkit.c |  2 +-
 sound/soc/blackfin/Kconfig              | 16 ++++++++--------
 sound/soc/blackfin/bf5xx-ad1836.c       | 18 +++++++-----------
 7 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/arch/blackfin/mach-bf527/boards/ezkit.c b/arch/blackfin/mach-bf527/boards/ezkit.c
index 29f16e5..8224895 100644
--- a/arch/blackfin/mach-bf527/boards/ezkit.c
+++ b/arch/blackfin/mach-bf527/boards/ezkit.c
< at >< at > -590,7 +590,7 < at >< at > static struct platform_device bfin_tdm = {
 #if defined(CONFIG_SND_BF5XX_SOC_AD1836) \
         || defined(CONFIG_SND_BF5XX_SOC_A

[PATCH 04/10] ASoC: blackfin: bf5xx-i2s:Allocate buffer only as large as requested

Lars-Peter Clausen — 2013-05-28T17:22:12

There is no need to always allocate the maximum buffer size. While we are at it
also pass errors returned by snd_pcm_lib_malloc_pages() on to the upper layers.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 sound/soc/blackfin/bf5xx-i2s-pcm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c
index 8726c3a..107c1c9 100644
--- a/sound/soc/blackfin/bf5xx-i2s-pcm.c
+++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c
< at >< at > -66,10 +66,7 < at >< at > static const struct snd_pcm_hardware bf5xx_pcm_hardware = {
 static int bf5xx_pcm_hw_params(struct snd_pcm_substream *substream,
 struct snd_pcm_hw_params *params)
 {
-size_t size = bf5xx_pcm_hardware.buffer_bytes_max;
-snd_pcm_lib_malloc_pages(substream, size);
-
-return 0;
+return snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(params));
 }
 
 static int bf5xx_pcm_hw_free(struct snd_pcm_substream *substream)

[PATCH 05/10] ASoC: blackfin: bf5xx-i2s-pcm:Use snd_pcm_lib_preallocate_pages_for_all()

Lars-Peter Clausen — 2013-05-28T17:22:13

Use snd_pcm_lib_preallocate_pages_for_all() for pre-allocating the DMA buffers
instead of re-implementing the same functionality.  Note that there is no need
to call snd_pcm_lib_free_pages_for_all() since the ALSA core takes care of this
for us.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 sound/soc/blackfin/bf5xx-i2s-pcm.c | 63 ++------------------------------------
 1 file changed, 3 insertions(+), 60 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c
index 107c1c9..9931a18 100644
--- a/sound/soc/blackfin/bf5xx-i2s-pcm.c
+++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c
< at >< at > -209,55 +209,12 < at >< at > static struct snd_pcm_ops bf5xx_pcm_i2s_ops = {
 .mmap= bf5xx_pcm_mmap,
 };
 
-static int bf5xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
-{
-struct snd_pcm_substream *substream = pcm->streams[stream].substream;
-struct snd_dma_buffer *buf = &substream->dma_buffer;
-size_t size = bf5xx_pcm_hardware.buffer_bytes_max;

[PATCH 02/10] ASoC: blackfin: bf5xx-i2s: Usedev_{err, dbg} instead of pr_{error, debug}

Lars-Peter Clausen — 2013-05-28T17:22:10

Using dev_{err,dbg} instead of pr_{error,debug} makes it easier to recognize
which device created the message.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 sound/soc/blackfin/bf5xx-i2s.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-i2s.c b/sound/soc/blackfin/bf5xx-i2s.c
index dd0c2a4..78411f2 100644
--- a/sound/soc/blackfin/bf5xx-i2s.c
+++ b/sound/soc/blackfin/bf5xx-i2s.c
< at >< at > -74,7 +74,8 < at >< at > static int bf5xx_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 ret = -EINVAL;
 break;
 default:
-printk(KERN_ERR "%s: Unknown DAI format type\n", __func__);
+dev_err(cpu_dai->dev, "%s: Unknown DAI format type\n",
+__func__);
 ret = -EINVAL;
 break;
 }
< at >< at > -88,7 +89,8 < at >< at > static int bf5xx_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 ret = -EINVAL;
 break;
 default:
-printk(KERN_ERR "%s: Unknown DAI master type\n", __func__);
+dev_err(cpu_dai->dev, "%s: Unknown DAI master type\n",
+

[PATCH 06/10] ASoC: blackfin: bf5xx-i2s: Addsupport for TDM mode

Lars-Peter Clausen — 2013-05-28T17:22:14

The bf5xx-i2s{,-pcm} and bf5xx-tdm{-pcm} drivers are nearly identical. Both are
for the same hardware each supporting a slight different subset of the hardware.
The bf5xx-i2s driver supports 2 channel I2S mode while the bf5xx-tdm driver
supports TDM mode and channel remapping. This patch adds support for TDM mode
and channel remapping to the bf5xx-i2s driver so that we'll eventually be able
to retire the bf5xx-tdm driver. Unfortunately the hardware is fixed to using 8
channels in TDM mode. The bf5xx-tdm driver jumps through a few hoops to make it
work well with other channel counts as well:
* Don't support mmap
* Translate between internal frame size (which is always 8 * sample_size)
  and ALSA frame size (which depends on the channel count)
* Have special copy and silence callbacks which are aware of the mismatch
  between internal and ALSA frame size
* Reduce the maximum buffer size to ensure that there is enough headroom for
  dummy data.

The bf5xx-i2s driver is going to use the same mechanisms wh

[PATCH 03/10] ASoC: blackfin: bf5xx-sport:Allow setting rx and tx mask independently

Lars-Peter Clausen — 2013-05-28T17:22:11

Since the hardware supports it there is no need to artificially limit this to
just being able to set the same mask for both tx and rx.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw< at >public.gmane.org>
---
 sound/soc/blackfin/bf5xx-ac97.c  |  8 ++++----
 sound/soc/blackfin/bf5xx-sport.c | 10 +++++-----
 sound/soc/blackfin/bf5xx-sport.h |  2 +-
 sound/soc/blackfin/bf5xx-tdm.c   |  4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-ac97.c b/sound/soc/blackfin/bf5xx-ac97.c
index 4902173..c66bef8 100644
--- a/sound/soc/blackfin/bf5xx-ac97.c
+++ b/sound/soc/blackfin/bf5xx-ac97.c
< at >< at > -231,9 +231,9 < at >< at > static int bf5xx_ac97_resume(struct snd_soc_dai *dai)
 return 0;
 
 #if defined(CONFIG_SND_BF5XX_MULTICHAN_SUPPORT)
-ret = sport_set_multichannel(sport, 16, 0x3FF, 1);
+ret = sport_set_multichannel(sport, 16, 0x3FF, 0x3FF, 1);
 #else
-ret = sport_set_multichannel(sport, 16, 0x1F, 1);
+ret = sport_set_multichannel(sport, 16, 0x1F, 0x1F, 1);
 #endif
 if

Re: [PATCH] arch: configuration,deleting 'CONFIG_BUG' since always need it.

Arnd Bergmann — 2013-05-28T17:20:37

Yes, actually the return value has to be maintained as well.
The current (!CONFIG_BUG) default implementation is

#define WARN_ON(condition) ({                                           \
        int __ret_warn_on = !!(condition);                              \
        unlikely(__ret_warn_on);                                        \
})

which seems fine.

#define WARN_ON(condition) unlikely(!!(condition))

is probably just as good.

Arnd

Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

H. Peter Anvin — 2013-05-28T16:06:48

It is possible to define WARN_ON() as:

#define WARN_ON(x) ((void)(x))

... which preserves side effects.

-hpa

Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

H. Peter Anvin — 2013-05-28T14:55:32

I suspect, rather, that BUG() should turn into a trap (or jump to a
death routine) under any circumstances.  The one thing that can be
omitted for small configurations are the annotations, which only serve
to output a more human-readable error message.

-hpa

Re: [PATCH] arch: configuration,deleting 'CONFIG_BUG' since always need it.

Arnd Bergmann — 2013-05-23T12:09:02

I'm absolutely aware of this. Again, the current behaviour of doing nothing
at all isn't very different from undefined behavior when you get when you
get to the end of a function returning a pointer without a "return" statement,
or when you return from a function that has determined that it is not safe
to continue.


I did not claim that it was a good idea to disable BUG(), all I said is
that "random stuff may happen" is probably what Matt Mackall had in mind when
he introduced the option.

Arnd

Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.

Chen Gang — 2013-05-23T11:19:30

So I think neither unreachable() nor panic() are suitable for this
condition.

I guess 'CONFIG_BUG' is not belong to common features, now (and in the
future), so it is not suitable still exist in "asm-generic/bug.h", need
remove it firstly.

And then let the specific architectures to implement their own BUG(), if
they want some special features.

SO most of arches can skip this issue.


Thanks.

Re: [PATCH] arch: configuration,deleting 'CONFIG_BUG' sincealways need it.

Russell King - ARM Linux — 2013-05-23T10:29:34

Semantics of the code really don't come in to it if you use unreachable().
unreachable() is an effective do { } while (0) to the compiler.  It just
doesn't warn about it anymore.  It's actually worse than that - it's
permission to the compiler to just stop considering flow control at that
point and do anything it likes with the following instruction slot.

What __builtin_unreachable() means to the compiler is "we will *never*
get here".  That isn't the case for BUG() - BUG() means "we hope that
we will never get here, but we might, and if we do your data is in
grave danger."

We should either have something at that point (like a call to a function
which panics) or remove the ability to turn off CONFIG_BUG and anyone who
cares about kernel size needs to come up with a single trapping
instruction BUG() implementation.