All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Mack <daniel@caiaq.de>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel@alsa-project.org, eric.y.miao@gmail.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 16/17] ALSA: ARM: add Raumfeld audio support
Date: Wed, 25 Nov 2009 13:24:36 +0100	[thread overview]
Message-ID: <20091125122436.GI29442@buzzloop.caiaq.de> (raw)
In-Reply-To: <20091125110218.GD17420@opensource.wolfsonmicro.com>

On Wed, Nov 25, 2009 at 11:02:18AM +0000, Mark Brown wrote:
> 
> > +static struct i2c_board_info max9486_hwmon_info = {
> > +	I2C_BOARD_INFO("max9485", 0x63),
> > +};
> 
> This should be in the board file under arch/arm.

Well, the problem is that if I do it there, I don't get a handle for the
actual data transfer, which I now get from i2c_new_device(). There is no
driver matching this device (it wasn't taken because it's 'too simple').

How would I get a handle to pass to i2c_master_send() or an equivalent
function?

> > +extern void raumfeld_enable_audio(bool en);
> 
> This should be in a proper header file somewhere.

Hmm, I thought so too, but it would be the only thing to add there.
Hence I decided to not do that. You really prefer that?

> > +static void raumfeld_cs4270_shutdown(struct snd_pcm_substream *substream)
> > +{
> > +}
> > +
> 
> Remove this and the other empty functions.

Hmm, I tried that and it crashed the kernel. I will check again as that
was some month ago.

> > +	case 96000:
> > +		set_max9485_clk(1);
> 
> It might be a bit more legible to have some constants for the arguments
> for this function - it's not entirely clear what's going on, the 1 and 0
> look like a boolean but that's not what's really going on here.  It may
> be better to do this as a proper driver, there was at least one driver I
> remember being posted for a TDM clock generator which I think got
> merged.

No, it wasn't.

  http://marc.info/?l=linux-i2c&m=122457836326525&w=2

Jean Delvare's last comment on this was:

> Honestly I don't see any value in this driver. There's nothing you can
> do with it that you couldn't already do without it.

The driver itself would do the right thing, but I doubt that
resubmitting will help much.

Daniel

WARNING: multiple messages have this Message-ID (diff)
From: daniel@caiaq.de (Daniel Mack)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 16/17] ALSA: ARM: add Raumfeld audio support
Date: Wed, 25 Nov 2009 13:24:36 +0100	[thread overview]
Message-ID: <20091125122436.GI29442@buzzloop.caiaq.de> (raw)
In-Reply-To: <20091125110218.GD17420@opensource.wolfsonmicro.com>

On Wed, Nov 25, 2009 at 11:02:18AM +0000, Mark Brown wrote:
> 
> > +static struct i2c_board_info max9486_hwmon_info = {
> > +	I2C_BOARD_INFO("max9485", 0x63),
> > +};
> 
> This should be in the board file under arch/arm.

Well, the problem is that if I do it there, I don't get a handle for the
actual data transfer, which I now get from i2c_new_device(). There is no
driver matching this device (it wasn't taken because it's 'too simple').

How would I get a handle to pass to i2c_master_send() or an equivalent
function?

> > +extern void raumfeld_enable_audio(bool en);
> 
> This should be in a proper header file somewhere.

Hmm, I thought so too, but it would be the only thing to add there.
Hence I decided to not do that. You really prefer that?

> > +static void raumfeld_cs4270_shutdown(struct snd_pcm_substream *substream)
> > +{
> > +}
> > +
> 
> Remove this and the other empty functions.

Hmm, I tried that and it crashed the kernel. I will check again as that
was some month ago.

> > +	case 96000:
> > +		set_max9485_clk(1);
> 
> It might be a bit more legible to have some constants for the arguments
> for this function - it's not entirely clear what's going on, the 1 and 0
> look like a boolean but that's not what's really going on here.  It may
> be better to do this as a proper driver, there was at least one driver I
> remember being posted for a TDM clock generator which I think got
> merged.

No, it wasn't.

  http://marc.info/?l=linux-i2c&m=122457836326525&w=2

Jean Delvare's last comment on this was:

> Honestly I don't see any value in this driver. There's nothing you can
> do with it that you couldn't already do without it.

The driver itself would do the right thing, but I doubt that
resubmitting will help much.

Daniel

  reply	other threads:[~2009-11-25 12:24 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-25 10:42 ARM: pxa: add support for Raumfeld audio devices Daniel Mack
2009-11-25 10:42 ` [PATCH 01/17] ARM: pxa/raumfeld: add basic structure for devices Daniel Mack
2009-11-25 15:26   ` Mike Rapoport
2009-11-25 15:44     ` Daniel Mack
2009-11-25 16:51     ` Marek Vasut
2009-11-25 17:06       ` Russell King - ARM Linux
2009-11-25 10:42 ` [PATCH 02/17] ARM: pxa/raumfeld: add GPIO definitions Daniel Mack
2009-11-25 10:42 ` [PATCH 03/17] ARM: pxa/raumfeld: add SMSC9220 ethernet support Daniel Mack
2009-11-25 15:26   ` Mike Rapoport
2009-11-25 15:49     ` Daniel Mack
2009-11-25 16:56       ` Russell King - ARM Linux
2009-11-26 17:55         ` Daniel Mack
2009-11-25 10:42 ` [PATCH 04/17] ARM: pxa/raumfeld: add OHCI function Daniel Mack
2009-11-25 10:42 ` [PATCH 05/17] ARM: pxa/raumfeld: add rotary encoder support Daniel Mack
2009-11-25 15:26   ` Mike Rapoport
2009-11-25 15:50     ` Daniel Mack
2009-11-25 16:59     ` Russell King - ARM Linux
2009-11-25 10:42 ` [PATCH 06/17] ARM: pxa/raumfeld: add GPIO buttons Daniel Mack
2009-11-25 10:42 ` [PATCH 07/17] ARM: pxa/raumfeld: add GPIO connected LEDs Daniel Mack
2009-11-25 10:42 ` [PATCH 08/17] ARM: pxa/raumfeld: add one-wire function Daniel Mack
2009-11-25 17:02   ` Russell King - ARM Linux
2009-11-25 10:42 ` [PATCH 09/17] ARM: pxa/raumfeld: add NAND partitions Daniel Mack
2009-11-25 15:26   ` Mike Rapoport
2009-11-25 10:42 ` [PATCH 10/17] ARM: pxa/raumfeld: add framebuffer and backlight devices Daniel Mack
2009-11-25 10:42 ` [PATCH 11/17] ARM: pxa/raumfeld: add SPI controlled devices Daniel Mack
2009-11-25 10:42 ` [PATCH 12/17] ARM: pxa/raumfeld: add audio related functions Daniel Mack
2009-11-25 11:41   ` Mark Brown
2009-11-25 12:28     ` Daniel Mack
2009-11-25 13:07       ` Mark Brown
2009-11-25 13:53         ` Daniel Mack
2009-11-25 14:00           ` Mark Brown
2009-11-25 17:07   ` Russell King - ARM Linux
2009-11-25 10:42 ` [PATCH 13/17] ARM: pxa/raumfeld: add Marvell Libertas via SDIO Daniel Mack
2009-11-25 11:49   ` Mark Brown
2009-11-25 13:04     ` Daniel Mack
2009-11-25 13:36       ` Mark Brown
2009-11-25 10:42 ` [PATCH 14/17] ARM: pxa/raumfeld: add power supply framework Daniel Mack
2009-11-25 10:42 ` [PATCH 15/17] ARM: pxa/raumfeld: add support for I2C controlled devices Daniel Mack
2009-11-25 10:42 ` [PATCH 16/17] ALSA: ARM: add Raumfeld audio support Daniel Mack
2009-11-25 10:42   ` Daniel Mack
2009-11-25 11:02   ` Mark Brown
2009-11-25 11:02     ` Mark Brown
2009-11-25 12:24     ` Daniel Mack [this message]
2009-11-25 12:24       ` Daniel Mack
2009-11-25 13:29       ` Mark Brown
2009-11-25 13:29         ` Mark Brown
2009-11-26 17:51         ` Daniel Mack
2009-11-26 17:51           ` Daniel Mack
2009-11-25 10:42 ` [PATCH 17/17] ARM: pxa/raumfeld: Add defconfig Daniel Mack
2009-11-25 11:27 ` ARM: pxa: add support for Raumfeld audio devices Mike Rapoport
2009-11-25 12:14   ` Daniel Mack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091125122436.GI29442@buzzloop.caiaq.de \
    --to=daniel@caiaq.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=eric.y.miao@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.