Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev

2015-01-13 Thread Antti Palosaari 

On 01/11/2015 04:25 PM, Mauro Carvalho Chehab wrote:

Em Sun, 11 Jan 2015 16:02:41 +0200
Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu:



I'm not too familiar with tuners, do they all have a single output only and no
input ?


They have an input: the antenna connector. However, I don't see any need
to map it for most tuners, as there's generally just one input, hardwired
into the tuner chip.

There are some hardware with 2 antenna connectors, but for different
functions (FM and TV). They're selected automatically when the V4L2
driver switches between FM and TV.

In any case, the tuner-core doesn't provide any way to select the
antenna input.

So, if a driver would need to select the input, it would either need
to not use tuner-core or some patch will be required to add such
functionality inside tuner-core.


Tuner has antenna as a input and output is intermediate frequency or 
baseband (IF/BB (zero-IF)).


I think most modern silicon tuners actually has more than one physical 
antenna inputs - but those are left unused or same physical antenna 
connector is wired to all those inputs.


Sooner or later there will be receiver having multiple antenna 
connectors which are selectable by software. So let it be at least 
option easy to add later.


regards
Antti

--
http://palosaari.fi/
--
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 1/7] tuner-core: properly initialize media controller subdev

2015-01-11 Thread Laurent Pinchart 
Hi Mauro,

Thank you for the patch.

On Saturday 03 January 2015 18:09:33 Mauro Carvalho Chehab wrote:
 Properly initialize tuner core subdev at the media controller.
 
 That requires a new subtype at the media controller API.
 
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 
 diff --git a/drivers/media/v4l2-core/tuner-core.c
 b/drivers/media/v4l2-core/tuner-core.c index 559f8372e2eb..114715ed0110
 100644
 --- a/drivers/media/v4l2-core/tuner-core.c
 +++ b/drivers/media/v4l2-core/tuner-core.c
 @@ -134,6 +134,9 @@ struct tuner {
   unsigned inttype; /* chip type id */
   void*config;
   const char  *name;
 +#if defined(CONFIG_MEDIA_CONTROLLER)
 + struct media_padpad;
 +#endif

I'm not too familiar with tuners, do they all have a single output only and no 
input ?

  };
 
  /*
 @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int
 type, t-name = analog_ops-info.name;
   }
 
 + t-sd.entity.name = t-name;
 +

Entity information is not supposed to change at runtime, I'm not sure to be 
comfortable with this change.

set_type() is called at probe time and in tuner_s_type_addr(). The former just 
duplicates the name initialization in tuner_probe(), so isn't really needed. 
The later bothers me.

   tuner_dbg(type set to %s\n, t-name);
 
   t-mode_mask = new_mode_mask;
 @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
   struct tuner *t;
   struct tuner *radio;
   struct tuner *tv;
 + int ret;
 
   t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
   if (NULL == t)
 @@ -696,6 +702,15 @@ register_client:
  t-type,
  t-mode_mask  T_RADIO ?  Radio : ,
  t-mode_mask  T_ANALOG_TV ?  TV : );
 +#if defined(CONFIG_MEDIA_CONTROLLER)
 + t-pad.flags = MEDIA_PAD_FL_SOURCE;
 + t-sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
 + t-sd.entity.name = t-name;

v4l2_subdev_init(), called by v4l2_i2c_subdev_init(), sets sd-entity.name to 
point to sd-name. Is there a reason why the subdev name can't be used as the 
entity name ?

 +
 + ret = media_entity_init(t-sd.entity, 1, t-pad, 0);
 + if (ret  0)
 + tuner_err(failed to initialize media entity!\n);
 +#endif
   return 0;
  }
 
 diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
 index 707db275f92b..5ffde035789b 100644
 --- a/include/uapi/linux/media.h
 +++ b/include/uapi/linux/media.h
 @@ -66,6 +66,8 @@ struct media_device_info {
  /* A converter of analogue video to its digital representation. */
  #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER  (MEDIA_ENT_T_V4L2_SUBDEV + 4)
 
 +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER(MEDIA_ENT_T_V4L2_SUBDEV + 5)
 +
  #define MEDIA_ENT_FL_DEFAULT (1  0)
 
  struct media_entity_desc {

-- 
Regards,

Laurent Pinchart

--
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 1/7] tuner-core: properly initialize media controller subdev

2015-01-11 Thread Mauro Carvalho Chehab 
Em Sun, 11 Jan 2015 16:02:41 +0200
Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu:

 Hi Mauro,
 
 Thank you for the patch.
 
 On Saturday 03 January 2015 18:09:33 Mauro Carvalho Chehab wrote:
  Properly initialize tuner core subdev at the media controller.
  
  That requires a new subtype at the media controller API.
  
  Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
  
  diff --git a/drivers/media/v4l2-core/tuner-core.c
  b/drivers/media/v4l2-core/tuner-core.c index 559f8372e2eb..114715ed0110
  100644
  --- a/drivers/media/v4l2-core/tuner-core.c
  +++ b/drivers/media/v4l2-core/tuner-core.c
  @@ -134,6 +134,9 @@ struct tuner {
  unsigned inttype; /* chip type id */
  void*config;
  const char  *name;
  +#if defined(CONFIG_MEDIA_CONTROLLER)
  +   struct media_padpad;
  +#endif
 
 I'm not too familiar with tuners, do they all have a single output only and 
 no 
 input ?

They have an input: the antenna connector. However, I don't see any need
to map it for most tuners, as there's generally just one input, hardwired
into the tuner chip.

There are some hardware with 2 antenna connectors, but for different
functions (FM and TV). They're selected automatically when the V4L2
driver switches between FM and TV.

In any case, the tuner-core doesn't provide any way to select the
antenna input.

So, if a driver would need to select the input, it would either need
to not use tuner-core or some patch will be required to add such
functionality inside tuner-core.

   };
  
   /*
  @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int
  type, t-name = analog_ops-info.name;
  }
  
  +   t-sd.entity.name = t-name;
  +
 
 Entity information is not supposed to change at runtime, I'm not sure to be 
 comfortable with this change.
 
 set_type() is called at probe time and in tuner_s_type_addr(). The former 
 just 
 duplicates the name initialization in tuner_probe(), so isn't really needed. 
 The later bothers me.

The tuner-core driver is a core subdev implementation. It handles
the ioctl logic, but the actual driver is a different one. It also
have internally a probe logic that will load the correct tuner subdev.

The tuner_s_type_addr() callback, used only at bridge probing time,
is a way for the bridge driver to provide the name of the tuner driver
that should be loaded, plus its I2C address.

So, once the board is probed, the name shouldn't change.

 
  tuner_dbg(type set to %s\n, t-name);
  
  t-mode_mask = new_mode_mask;
  @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
  struct tuner *t;
  struct tuner *radio;
  struct tuner *tv;
  +   int ret;
  
  t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
  if (NULL == t)
  @@ -696,6 +702,15 @@ register_client:
 t-type,
 t-mode_mask  T_RADIO ?  Radio : ,
 t-mode_mask  T_ANALOG_TV ?  TV : );
  +#if defined(CONFIG_MEDIA_CONTROLLER)
  +   t-pad.flags = MEDIA_PAD_FL_SOURCE;
  +   t-sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
  +   t-sd.entity.name = t-name;
 
 v4l2_subdev_init(), called by v4l2_i2c_subdev_init(), sets sd-entity.name to 
 point to sd-name. Is there a reason why the subdev name can't be used as the 
 entity name ?

If we don't set entity.name to t-name, the sd-name will be tuner-core,
instead of the name of the real subdev.

  +
  +   ret = media_entity_init(t-sd.entity, 1, t-pad, 0);
  +   if (ret  0)
  +   tuner_err(failed to initialize media entity!\n);
  +#endif
  return 0;
   }
  
  diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
  index 707db275f92b..5ffde035789b 100644
  --- a/include/uapi/linux/media.h
  +++ b/include/uapi/linux/media.h
  @@ -66,6 +66,8 @@ struct media_device_info {
   /* A converter of analogue video to its digital representation. */
   #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER(MEDIA_ENT_T_V4L2_SUBDEV + 4)
  
  +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER  (MEDIA_ENT_T_V4L2_SUBDEV + 5)
  +
   #define MEDIA_ENT_FL_DEFAULT   (1  0)
  
   struct media_entity_desc {
 
--
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 1/7] tuner-core: properly initialize media controller subdev

2015-01-03 Thread Mauro Carvalho Chehab 
Properly initialize tuner core subdev at the media controller.

That requires a new subtype at the media controller API.

Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

diff --git a/drivers/media/v4l2-core/tuner-core.c 
b/drivers/media/v4l2-core/tuner-core.c
index 559f8372e2eb..114715ed0110 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -134,6 +134,9 @@ struct tuner {
unsigned inttype; /* chip type id */
void*config;
const char  *name;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_padpad;
+#endif
 };
 
 /*
@@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int 
type,
t-name = analog_ops-info.name;
}
 
+   t-sd.entity.name = t-name;
+
tuner_dbg(type set to %s\n, t-name);
 
t-mode_mask = new_mode_mask;
@@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
struct tuner *t;
struct tuner *radio;
struct tuner *tv;
+   int ret;
 
t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
if (NULL == t)
@@ -696,6 +702,15 @@ register_client:
   t-type,
   t-mode_mask  T_RADIO ?  Radio : ,
   t-mode_mask  T_ANALOG_TV ?  TV : );
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   t-pad.flags = MEDIA_PAD_FL_SOURCE;
+   t-sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
+   t-sd.entity.name = t-name;
+
+   ret = media_entity_init(t-sd.entity, 1, t-pad, 0);
+   if (ret  0)
+   tuner_err(failed to initialize media entity!\n);
+#endif
return 0;
 }
 
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 707db275f92b..5ffde035789b 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -66,6 +66,8 @@ struct media_device_info {
 /* A converter of analogue video to its digital representation. */
 #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER(MEDIA_ENT_T_V4L2_SUBDEV + 4)
 
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER  (MEDIA_ENT_T_V4L2_SUBDEV + 5)
+
 #define MEDIA_ENT_FL_DEFAULT   (1  0)
 
 struct media_entity_desc {
-- 
2.1.0

--
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 1/7] tuner-core: properly initialize media controller subdev

2015-01-03 Thread Sakari Ailus 

Hi Mauro,

Mauro Carvalho Chehab wrote:

Properly initialize tuner core subdev at the media controller.

That requires a new subtype at the media controller API.

Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

diff --git a/drivers/media/v4l2-core/tuner-core.c 
b/drivers/media/v4l2-core/tuner-core.c
index 559f8372e2eb..114715ed0110 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -134,6 +134,9 @@ struct tuner {
unsigned inttype; /* chip type id */
void*config;
const char  *name;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_padpad;
+#endif
  };

  /*
@@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int 
type,
t-name = analog_ops-info.name;
}

+   t-sd.entity.name = t-name;
+
tuner_dbg(type set to %s\n, t-name);

t-mode_mask = new_mode_mask;
@@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
struct tuner *t;
struct tuner *radio;
struct tuner *tv;
+   int ret;


This will emit a compiler warning if CONFIG_MEDIA_CONTROLLER isn't defined.


t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
if (NULL == t)
@@ -696,6 +702,15 @@ register_client:
   t-type,
   t-mode_mask  T_RADIO ?  Radio : ,
   t-mode_mask  T_ANALOG_TV ?  TV : );
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   t-pad.flags = MEDIA_PAD_FL_SOURCE;
+   t-sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
+   t-sd.entity.name = t-name;
+
+   ret = media_entity_init(t-sd.entity, 1, t-pad, 0);
+   if (ret  0)
+   tuner_err(failed to initialize media entity!\n);


I might return the error back to the caller. The failing initialisation 
of a media entity itself might not be a fatal problem, but someone later 
assuming it has been initialised might be.



+#endif
return 0;
  }

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 707db275f92b..5ffde035789b 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -66,6 +66,8 @@ struct media_device_info {
  /* A converter of analogue video to its digital representation. */
  #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER   (MEDIA_ENT_T_V4L2_SUBDEV + 4)

+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER  (MEDIA_ENT_T_V4L2_SUBDEV + 5)
+
  #define MEDIA_ENT_FL_DEFAULT  (1  0)

  struct media_entity_desc {



--
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com
--
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