Re: [PATCH 0/3] Driver for TI WL1273 FM radio.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Hans,

thank you for the comments.

On Fri, 2010-04-23 at 11:16 +0200, ext Hans Verkuil wrote:
> On Tuesday 20 April 2010 17:20:04 Matti J. Aaltonen wrote:
> > Hi.
> > 
> > This is the initial version of my driver for Texas Instruments
> > WL1273 FM receiver transmitter. The driver is divided into three parts:
> > the MFD core which handles the communication with the chip and also
> > keeps the chip state, ASoC codec takes care of the digital audio part and
> > the V4L2 control part with some private IOCTLs.
> > 
> > This is my first up-streaming effort so all comments are welcome.
> 
> OK, I did a quick review and the main things that you need to look at are
> the RDS receiver API as defined in the spec
> (http://www.linuxtv.org/downloads/v4l-dvb-apis/ch04s11.html) and the FM and
> RDS transmitter controls:
> http://www.linuxtv.org/downloads/v4l-dvb-apis/ch01s09.html#fm-tx-controls.

OK, I'll do that.

> 
> Any private controls that you think you need should be discussed first. We
> may need to standardize them.

OK, I didn't realize that. But it makes sense, so let's discuss...

> The other thing you have to do in the V4L2 driver is to use struct v4l2_device.
> See also Documentation/video4linux/v4l2-framework.txt.

OK.

> I also noticed some FM and RDS things in the alsa driver. It is not clear to
> me why these are there since this is pretty much V4L2 specific.

Yes, I kind of new that those controls weren't a good idea. But I
implemented those when I didn't know better. Ande I left them there to
see if they would get accepted. But I'll remove them as the
functionality is duplicated in the V4L2 module.

> Regarding hardcoding regions: isn't this more for the application? Are there
> any legal requirements for region handling?

I thought about that a lot. And I know that the regions are a policy
thing that doesn't belong into the driver/kernel. But those two regions
are directly supported by the hardware so I thought that it would be OK
make them available.

> Most radio tuners just accept the whole frequency range that they support and
> leave it to the application to restrict it if needed depending on the region.

I understand... see above...

> Those disabled controls like bass, treble etc. should be removed. Workarounds
> for plainly broken applications is not something we want in our drivers.
> Instead make a patch for that app and send it to the maintainer. If it is
> unmaintained, then let us know: we can move unmaintained but frequently used
> apps to our own repository.

Fine... I did that just to copy the functionality of some existing
driver when I started.

B.R.

Matti

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Cheers,
> > Matti
> > 
> > Matti J. Aaltonen (3):
> >   MFD: WL1273 FM Radio: MFD driver for the FM radio.
> >   ASoC: WL1273 FM Radio: Digital audio codec.
> >   V4L2: WL1273 FM Radio: Controls for the FM radio.
> > 
> >  drivers/media/radio/Kconfig        |   15 +
> >  drivers/media/radio/Makefile       |    1 +
> >  drivers/media/radio/radio-wl1273.c |  805 ++++++++++++++++
> >  drivers/mfd/Kconfig                |    6 +
> >  drivers/mfd/Makefile               |    2 +
> >  drivers/mfd/wl1273-core.c          | 1825 ++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/wl1273-core.h    |  265 ++++++
> >  sound/soc/codecs/Kconfig           |    6 +
> >  sound/soc/codecs/Makefile          |    2 +
> >  sound/soc/codecs/wl1273.c          |  708 ++++++++++++++
> >  sound/soc/codecs/wl1273.h          |   49 +
> >  11 files changed, 3684 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/media/radio/radio-wl1273.c
> >  create mode 100644 drivers/mfd/wl1273-core.c
> >  create mode 100644 include/linux/mfd/wl1273-core.h
> >  create mode 100644 sound/soc/codecs/wl1273.c
> >  create mode 100644 sound/soc/codecs/wl1273.h
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> 


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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux