Re: [PATCH] radio-bcm2048: Enable access to automute and ctrl registers

2015-06-06 Thread Pavel Machek
On Fri 2015-06-05 13:34:01, Hans Verkuil wrote:
 On 05/15/2015 11:31 PM, Pali Rohár wrote:
  From: maxx m...@spaceboyz.net
  
  This enables access to automute function of the chip via sysfs and
  gives direct access to FM_AUDIO_CTRL0/1 registers, also via sysfs. I
  don't think this is so important but helps in developing radio scanner
  apps.
  
  Patch writen by m...@spaceboyz.net
  
  Signed-off-by: Pali Rohár pali.ro...@gmail.com
  Cc: m...@spaceboyz.net
 
 As Pavel mentioned, these patches need to be resend with correct Signed-off-by
 lines.
 
 Regarding this patch: I don't want to apply this since this really should be a
 control. Or just enable it always. If someone wants to make this a control, 
 then
 let me know: there are two other drivers with an AUTOMUTE control: bttv and 
 saa7134.
 
 In both cases it is implemented as a private control, but it makes sense to
 promote this to a standard user control. I can make a patch for that.
 
 And for CTRL0/1: if you want direct register access, then implement
 VIDIOC_DBG_G/S_REGISTER. This makes sure you have the right permissions etc.
 
 More importantly: is anyone working on getting this driver out of staging? 
 It's
 been here for about a year and a half and I haven't seen any efforts to clean 
 it up.

Yes, there are. Unfortunately, this one depends on bluetooth driver, and we 
have some
fun with that one. So please be patient...


Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] radio-bcm2048: Enable access to automute and ctrl registers

2015-06-05 Thread Hans Verkuil
On 05/15/2015 11:31 PM, Pali Rohár wrote:
 From: maxx m...@spaceboyz.net
 
 This enables access to automute function of the chip via sysfs and
 gives direct access to FM_AUDIO_CTRL0/1 registers, also via sysfs. I
 don't think this is so important but helps in developing radio scanner
 apps.
 
 Patch writen by m...@spaceboyz.net
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com
 Cc: m...@spaceboyz.net

As Pavel mentioned, these patches need to be resend with correct Signed-off-by
lines.

Regarding this patch: I don't want to apply this since this really should be a
control. Or just enable it always. If someone wants to make this a control, then
let me know: there are two other drivers with an AUTOMUTE control: bttv and 
saa7134.

In both cases it is implemented as a private control, but it makes sense to
promote this to a standard user control. I can make a patch for that.

And for CTRL0/1: if you want direct register access, then implement
VIDIOC_DBG_G/S_REGISTER. This makes sure you have the right permissions etc.

More importantly: is anyone working on getting this driver out of staging? It's
been here for about a year and a half and I haven't seen any efforts to clean 
it up.

The whole sysfs part is out-of-spec and is another reason why I won't apply this
patch as is. Adding code to things that should go away isn't good...

So:

Nacked-by: Hans Verkuil hans.verk...@cisco.com

Regards,

Hans

 ---
  drivers/staging/media/bcm2048/radio-bcm2048.c |   96 
 +
  1 file changed, 96 insertions(+)
 
 diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
 b/drivers/staging/media/bcm2048/radio-bcm2048.c
 index 1482d4b..8f9ba7b 100644
 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
 +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
 @@ -826,6 +826,93 @@ static int bcm2048_get_mute(struct bcm2048_device *bdev)
   return err;
  }
  
 +static int bcm2048_set_automute(struct bcm2048_device *bdev, u8 automute)
 +{
 + int err;
 +
 + mutex_lock(bdev-mutex);
 +
 + err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_PAUSE, automute);
 +
 + mutex_unlock(bdev-mutex);
 + return err;
 +}
 +
 +static int bcm2048_get_automute(struct bcm2048_device *bdev)
 +{
 + int err;
 + u8 value;
 +
 + mutex_lock(bdev-mutex);
 +
 + err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_PAUSE, value);
 +
 + mutex_unlock(bdev-mutex);
 +
 + if (!err)
 + err = value;
 +
 + return err;
 +}
 +
 +static int bcm2048_set_ctrl0(struct bcm2048_device *bdev, u8 value)
 +{
 + int err;
 +
 + mutex_lock(bdev-mutex);
 +
 + err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL0, value);
 +
 + mutex_unlock(bdev-mutex);
 + return err;
 +}
 +
 +static int bcm2048_set_ctrl1(struct bcm2048_device *bdev, u8 value)
 +{
 + int err;
 +
 + mutex_lock(bdev-mutex);
 +
 + err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL1, value);
 +
 + mutex_unlock(bdev-mutex);
 + return err;
 +}
 +
 +static int bcm2048_get_ctrl0(struct bcm2048_device *bdev)
 +{
 + int err;
 + u8 value;
 +
 + mutex_lock(bdev-mutex);
 +
 + err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL0, value);
 +
 + mutex_unlock(bdev-mutex);
 +
 + if (!err)
 + err = value;
 +
 + return err;
 +}
 +
 +static int bcm2048_get_ctrl1(struct bcm2048_device *bdev)
 +{
 + int err;
 + u8 value;
 +
 + mutex_lock(bdev-mutex);
 +
 + err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL1, value);
 +
 + mutex_unlock(bdev-mutex);
 +
 + if (!err)
 + err = value;
 +
 + return err;
 +}
 +
  static int bcm2048_set_audio_route(struct bcm2048_device *bdev, u8 route)
  {
   int err;
 @@ -2058,6 +2145,9 @@ static ssize_t bcm2048_##prop##_read(struct device 
 *dev,\
  
  DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, %u, 0)
  DEFINE_SYSFS_PROPERTY(mute, unsigned, int, %u, 0)
 +DEFINE_SYSFS_PROPERTY(automute, unsigned, int, %x, 0)
 +DEFINE_SYSFS_PROPERTY(ctrl0, unsigned, int, %x, 0)
 +DEFINE_SYSFS_PROPERTY(ctrl1, unsigned, int, %x, 0)
  DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, %u, 0)
  DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, %u, 0)
  
 @@ -2095,6 +2185,12 @@ static struct device_attribute attrs[] = {
   bcm2048_power_state_write),
   __ATTR(mute, S_IRUGO | S_IWUSR, bcm2048_mute_read,
   bcm2048_mute_write),
 + __ATTR(automute, S_IRUGO | S_IWUSR, bcm2048_automute_read,
 + bcm2048_automute_write),
 + __ATTR(ctrl0, S_IRUGO | S_IWUSR, bcm2048_ctrl0_read,
 + bcm2048_ctrl0_write),
 + __ATTR(ctrl1, S_IRUGO | S_IWUSR, bcm2048_ctrl1_read,
 + bcm2048_ctrl1_write),
   __ATTR(audio_route, S_IRUGO | S_IWUSR, bcm2048_audio_route_read,
   bcm2048_audio_route_write),
   __ATTR(dac_output, S_IRUGO | S_IWUSR, bcm2048_dac_output_read,
 

--
To unsubscribe from this list: 

Re: [PATCH] radio-bcm2048: Enable access to automute and ctrl registers

2015-05-18 Thread Pavel Machek
On Fri 2015-05-15 23:31:51, Pali Rohár wrote:
 From: maxx m...@spaceboyz.net
 
 This enables access to automute function of the chip via sysfs and
 gives direct access to FM_AUDIO_CTRL0/1 registers, also via sysfs. I
 don't think this is so important but helps in developing radio scanner
 apps.

Is directly exposing hardware registers in sysfs a good idea?
(Debugfs?)

If this goes to sysfs, could we get interface description?


Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] radio-bcm2048: Enable access to automute and ctrl registers

2015-05-15 Thread Greg Kroah-Hartman
On Fri, May 15, 2015 at 11:31:51PM +0200, Pali Rohár wrote:
 From: maxx m...@spaceboyz.net

Same here, real name please.

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


[PATCH] radio-bcm2048: Enable access to automute and ctrl registers

2015-05-15 Thread Pali Rohár
From: maxx m...@spaceboyz.net

This enables access to automute function of the chip via sysfs and
gives direct access to FM_AUDIO_CTRL0/1 registers, also via sysfs. I
don't think this is so important but helps in developing radio scanner
apps.

Patch writen by m...@spaceboyz.net

Signed-off-by: Pali Rohár pali.ro...@gmail.com
Cc: m...@spaceboyz.net
---
 drivers/staging/media/bcm2048/radio-bcm2048.c |   96 +
 1 file changed, 96 insertions(+)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 1482d4b..8f9ba7b 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -826,6 +826,93 @@ static int bcm2048_get_mute(struct bcm2048_device *bdev)
return err;
 }
 
+static int bcm2048_set_automute(struct bcm2048_device *bdev, u8 automute)
+{
+   int err;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_PAUSE, automute);
+
+   mutex_unlock(bdev-mutex);
+   return err;
+}
+
+static int bcm2048_get_automute(struct bcm2048_device *bdev)
+{
+   int err;
+   u8 value;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_PAUSE, value);
+
+   mutex_unlock(bdev-mutex);
+
+   if (!err)
+   err = value;
+
+   return err;
+}
+
+static int bcm2048_set_ctrl0(struct bcm2048_device *bdev, u8 value)
+{
+   int err;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL0, value);
+
+   mutex_unlock(bdev-mutex);
+   return err;
+}
+
+static int bcm2048_set_ctrl1(struct bcm2048_device *bdev, u8 value)
+{
+   int err;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_send_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL1, value);
+
+   mutex_unlock(bdev-mutex);
+   return err;
+}
+
+static int bcm2048_get_ctrl0(struct bcm2048_device *bdev)
+{
+   int err;
+   u8 value;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL0, value);
+
+   mutex_unlock(bdev-mutex);
+
+   if (!err)
+   err = value;
+
+   return err;
+}
+
+static int bcm2048_get_ctrl1(struct bcm2048_device *bdev)
+{
+   int err;
+   u8 value;
+
+   mutex_lock(bdev-mutex);
+
+   err = bcm2048_recv_command(bdev, BCM2048_I2C_FM_AUDIO_CTRL1, value);
+
+   mutex_unlock(bdev-mutex);
+
+   if (!err)
+   err = value;
+
+   return err;
+}
+
 static int bcm2048_set_audio_route(struct bcm2048_device *bdev, u8 route)
 {
int err;
@@ -2058,6 +2145,9 @@ static ssize_t bcm2048_##prop##_read(struct device *dev,  
\
 
 DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, %u, 0)
 DEFINE_SYSFS_PROPERTY(mute, unsigned, int, %u, 0)
+DEFINE_SYSFS_PROPERTY(automute, unsigned, int, %x, 0)
+DEFINE_SYSFS_PROPERTY(ctrl0, unsigned, int, %x, 0)
+DEFINE_SYSFS_PROPERTY(ctrl1, unsigned, int, %x, 0)
 DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, %u, 0)
 DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, %u, 0)
 
@@ -2095,6 +2185,12 @@ static struct device_attribute attrs[] = {
bcm2048_power_state_write),
__ATTR(mute, S_IRUGO | S_IWUSR, bcm2048_mute_read,
bcm2048_mute_write),
+   __ATTR(automute, S_IRUGO | S_IWUSR, bcm2048_automute_read,
+   bcm2048_automute_write),
+   __ATTR(ctrl0, S_IRUGO | S_IWUSR, bcm2048_ctrl0_read,
+   bcm2048_ctrl0_write),
+   __ATTR(ctrl1, S_IRUGO | S_IWUSR, bcm2048_ctrl1_read,
+   bcm2048_ctrl1_write),
__ATTR(audio_route, S_IRUGO | S_IWUSR, bcm2048_audio_route_read,
bcm2048_audio_route_write),
__ATTR(dac_output, S_IRUGO | S_IWUSR, bcm2048_dac_output_read,
-- 
1.7.9.5

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