[PATCH v4 0/5] WL1273 FM Radio driver.

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

 



Hello again,

and thank you for the comments. 

New in this version of the patch set:

General headers:

I removed the seek level stuff and added the FM RX class. And I've added the
BAND IOCTL and I defined the three existing bands: I also added the OIRT band
because I think it's nicer to have three bands than only two.


Hans wrote:
> Is there a difference in power consumption between the 'off' and 'suspend' state
> of the device? I assume that 'off' has the lowest power consumption.

> In that case I would have the driver go to suspend when no one has opened the
> radioX device. And if the driver is asked to go to suspend (that's the linux
> suspend state), then the driver can turn off the radio and on resume it will
> reload the firmware.

> Does that sound reasonable?

Yes that's reasonable... But I have had one problem here: the "suspend" does not
work, but we are working on it.


> As mentioned I don't think the level stuff should be added at the moment.
> The spacing field is no problem, but don't forget to update the V4L2 spec as
> well. Also document there what should happen if spacing == 0 (which is the
> case for existing apps). It basically boils down to the fact that the driver
> uses the spacing as a hint only and will adjust it to whatever the hardware
> supports.

I've fixed the spacing handling.

I've dropped the seek level stuff.

drivers/mfd/wl1273-core.c:

> Don't use bitfields! How bitfields are ordered is compiler specific.

I've dropped the bitfields.

> Does the data you copy here conform to the v4l2_rds_data struct?
> In particular the block byte. It is well documented in the Spec in the
> section on 'Reading RDS data'.

The error bits are not same as in the spec so I now copy them to the correct
positions and use the v4l2_rds_data struct.


drivers/media/radio/radio-wl1273.c:

> I understood that the band was relevant for receiving only?

That's true, I've removed the band stuff here.

> Rather than continually allocating and freeing this buffer I would just make
> it a field in struct wl1273_device. It's never more than 256 bytes, so that's
> no problem.

Now the buffer gets allocated and freed only once.

> Don't. Just make sure there can be only one reader. This is the same principle
> used by video: there can be multiple file handles open on a video node, but
> only one can be used for streaming at a time. Trying to handle multiple readers
> or writers in a driver will lead to chaos. And this can be done much better by
> userspace.

Now only one reader or writer is accepted.

> Where are the other FM_TX controls?

Added the missing controls. However, there's read-only control - I didn't add that.

> Use strlcpy here as well.

Now I'm using strlcpy everywhere...

> I strongly recommend using v4l2_ctrl_query_fill() instead (defined in
> v4l2-common.c). This ensures consistent naming. It will also make it easier
> to convert to the upcoming new control framework.

Replaced the old code with code that uses v4l2_ctrl_query_fill etc.


> Make this dev_dbg. I think it is probably better to pick the closest spacing
> rather than falling back to 50.

I've changed spacing handling quite a bit.

Cheers,


Matti J. Aaltonen (5):
  V4L2: Add seek spacing and FM RX class.
  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.
  Documentation: v4l: Add hw_seek spacing.

 .../DocBook/v4l/vidioc-s-hw-freq-seek.xml          |   10 +-
 drivers/media/radio/Kconfig                        |   15 +
 drivers/media/radio/Makefile                       |    1 +
 drivers/media/radio/radio-wl1273.c                 | 1907 ++++++++++++++++++++
 drivers/mfd/Kconfig                                |    6 +
 drivers/mfd/Makefile                               |    2 +
 drivers/mfd/wl1273-core.c                          |  616 +++++++
 include/linux/mfd/wl1273-core.h                    |  326 ++++
 include/linux/videodev2.h                          |   15 +-
 sound/soc/codecs/Kconfig                           |    6 +
 sound/soc/codecs/Makefile                          |    2 +
 sound/soc/codecs/wl1273.c                          |  594 ++++++
 sound/soc/codecs/wl1273.h                          |   40 +
 13 files changed, 3537 insertions(+), 3 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


[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