All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] media: helene: add I2C device probe function
@ 2018-07-02  5:12 ` Katsuhiro Suzuki
  0 siblings, 0 replies; 4+ messages in thread
From: Katsuhiro Suzuki @ 2018-07-02  5:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, linux-media
  Cc: Sumit Semwal, Masami Hiramatsu, Jassi Brar, linux-arm-kernel,
	linux-kernel, Katsuhiro Suzuki

This patch adds I2C probe function to use dvb_module_probe()
with this driver.

Signed-off-by: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>

---

Changes since v3:
  - Drop wrong patch 2/2 from v3 series, no changes in this patch

Changes since v2:
  - Nothing

Changes since v1:
  - Add documents for dvb_frontend member of helene_config
---
 drivers/media/dvb-frontends/helene.c | 88 ++++++++++++++++++++++++++--
 drivers/media/dvb-frontends/helene.h |  3 +
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
index a0d0b53c91d7..04033f0c278b 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
 	return 0;
 }
 
-static int helene_set_params(struct dvb_frontend *fe)
+static int helene_set_params_t(struct dvb_frontend *fe)
 {
 	u8 data[MAX_WRITE_REGSIZE];
 	u32 frequency;
@@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
 	return 0;
 }
 
+static int helene_set_params(struct dvb_frontend *fe)
+{
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+
+	if (p->delivery_system == SYS_DVBT ||
+	    p->delivery_system == SYS_DVBT2 ||
+	    p->delivery_system == SYS_ISDBT ||
+	    p->delivery_system == SYS_DVBC_ANNEX_A)
+		return helene_set_params_t(fe);
+
+	return helene_set_params_s(fe);
+}
+
 static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 {
 	struct helene_priv *priv = fe->tuner_priv;
@@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 	return 0;
 }
 
-static const struct dvb_tuner_ops helene_tuner_ops = {
+static const struct dvb_tuner_ops helene_tuner_ops_t = {
 	.info = {
 		.name = "Sony HELENE Ter tuner",
 		.frequency_min = 1000000,
@@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
 	.init = helene_init,
 	.release = helene_release,
 	.sleep = helene_sleep,
-	.set_params = helene_set_params,
+	.set_params = helene_set_params_t,
 	.get_frequency = helene_get_frequency,
 };
 
@@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
 	.get_frequency = helene_get_frequency,
 };
 
+static const struct dvb_tuner_ops helene_tuner_ops = {
+	.info = {
+		.name = "Sony HELENE Sat/Ter tuner",
+		.frequency_min = 500000,
+		.frequency_max = 1200000000,
+		.frequency_step = 1000,
+	},
+	.init = helene_init,
+	.release = helene_release,
+	.sleep = helene_sleep,
+	.set_params = helene_set_params,
+	.get_frequency = helene_get_frequency,
+};
+
 /* power-on tuner
  * call once after reset
  */
@@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 0);
 
-	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops,
+	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops_t,
 			sizeof(struct dvb_tuner_ops));
 	fe->tuner_priv = priv;
 	dev_info(&priv->i2c->dev,
@@ -1042,6 +1069,59 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 }
 EXPORT_SYMBOL(helene_attach);
 
+static int helene_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct helene_config *config = client->dev.platform_data;
+	struct dvb_frontend *fe = config->fe;
+	struct device *dev = &client->dev;
+	struct helene_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->i2c_address = client->addr;
+	priv->i2c = client->adapter;
+	priv->set_tuner_data = config->set_tuner_priv;
+	priv->set_tuner = config->set_tuner_callback;
+	priv->xtal = config->xtal;
+
+	if (fe->ops.i2c_gate_ctrl)
+		fe->ops.i2c_gate_ctrl(fe, 1);
+
+	if (helene_x_pon(priv) != 0)
+		return -EINVAL;
+
+	if (fe->ops.i2c_gate_ctrl)
+		fe->ops.i2c_gate_ctrl(fe, 0);
+
+	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops,
+	       sizeof(struct dvb_tuner_ops));
+	fe->tuner_priv = priv;
+	i2c_set_clientdata(client, priv);
+
+	dev_info(dev, "Sony HELENE attached on addr=%x at I2C adapter %p\n",
+		 priv->i2c_address, priv->i2c);
+
+	return 0;
+}
+
+static const struct i2c_device_id helene_id[] = {
+	{ "helene", },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, helene_id);
+
+static struct i2c_driver helene_driver = {
+	.driver = {
+		.name = "helene",
+	},
+	.probe    = helene_probe,
+	.id_table = helene_id,
+};
+module_i2c_driver(helene_driver);
+
 MODULE_DESCRIPTION("Sony HELENE Sat/Ter tuner driver");
 MODULE_AUTHOR("Abylay Ospan <aospan@netup.ru>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/helene.h b/drivers/media/dvb-frontends/helene.h
index c9fc81c7e4e7..8562d01bc93e 100644
--- a/drivers/media/dvb-frontends/helene.h
+++ b/drivers/media/dvb-frontends/helene.h
@@ -39,6 +39,7 @@ enum helene_xtal {
  * @set_tuner_callback:	Callback function that notifies the parent driver
  *			which tuner is active now
  * @xtal: Cristal frequency as described by &enum helene_xtal
+ * @fe: Frontend for which connects this tuner
  */
 struct helene_config {
 	u8	i2c_address;
@@ -46,6 +47,8 @@ struct helene_config {
 	void	*set_tuner_priv;
 	int	(*set_tuner_callback)(void *, int);
 	enum helene_xtal xtal;
+
+	struct dvb_frontend *fe;
 };
 
 #if IS_REACHABLE(CONFIG_DVB_HELENE)
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v4] media: helene: add I2C device probe function
@ 2018-07-02  5:12 ` Katsuhiro Suzuki
  0 siblings, 0 replies; 4+ messages in thread
From: Katsuhiro Suzuki @ 2018-07-02  5:12 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds I2C probe function to use dvb_module_probe()
with this driver.

Signed-off-by: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>

---

Changes since v3:
  - Drop wrong patch 2/2 from v3 series, no changes in this patch

Changes since v2:
  - Nothing

Changes since v1:
  - Add documents for dvb_frontend member of helene_config
---
 drivers/media/dvb-frontends/helene.c | 88 ++++++++++++++++++++++++++--
 drivers/media/dvb-frontends/helene.h |  3 +
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
index a0d0b53c91d7..04033f0c278b 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
 	return 0;
 }
 
-static int helene_set_params(struct dvb_frontend *fe)
+static int helene_set_params_t(struct dvb_frontend *fe)
 {
 	u8 data[MAX_WRITE_REGSIZE];
 	u32 frequency;
@@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
 	return 0;
 }
 
+static int helene_set_params(struct dvb_frontend *fe)
+{
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+
+	if (p->delivery_system == SYS_DVBT ||
+	    p->delivery_system == SYS_DVBT2 ||
+	    p->delivery_system == SYS_ISDBT ||
+	    p->delivery_system == SYS_DVBC_ANNEX_A)
+		return helene_set_params_t(fe);
+
+	return helene_set_params_s(fe);
+}
+
 static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 {
 	struct helene_priv *priv = fe->tuner_priv;
@@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 	return 0;
 }
 
-static const struct dvb_tuner_ops helene_tuner_ops = {
+static const struct dvb_tuner_ops helene_tuner_ops_t = {
 	.info = {
 		.name = "Sony HELENE Ter tuner",
 		.frequency_min = 1000000,
@@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
 	.init = helene_init,
 	.release = helene_release,
 	.sleep = helene_sleep,
-	.set_params = helene_set_params,
+	.set_params = helene_set_params_t,
 	.get_frequency = helene_get_frequency,
 };
 
@@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
 	.get_frequency = helene_get_frequency,
 };
 
+static const struct dvb_tuner_ops helene_tuner_ops = {
+	.info = {
+		.name = "Sony HELENE Sat/Ter tuner",
+		.frequency_min = 500000,
+		.frequency_max = 1200000000,
+		.frequency_step = 1000,
+	},
+	.init = helene_init,
+	.release = helene_release,
+	.sleep = helene_sleep,
+	.set_params = helene_set_params,
+	.get_frequency = helene_get_frequency,
+};
+
 /* power-on tuner
  * call once after reset
  */
@@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 0);
 
-	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops,
+	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops_t,
 			sizeof(struct dvb_tuner_ops));
 	fe->tuner_priv = priv;
 	dev_info(&priv->i2c->dev,
@@ -1042,6 +1069,59 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 }
 EXPORT_SYMBOL(helene_attach);
 
+static int helene_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct helene_config *config = client->dev.platform_data;
+	struct dvb_frontend *fe = config->fe;
+	struct device *dev = &client->dev;
+	struct helene_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->i2c_address = client->addr;
+	priv->i2c = client->adapter;
+	priv->set_tuner_data = config->set_tuner_priv;
+	priv->set_tuner = config->set_tuner_callback;
+	priv->xtal = config->xtal;
+
+	if (fe->ops.i2c_gate_ctrl)
+		fe->ops.i2c_gate_ctrl(fe, 1);
+
+	if (helene_x_pon(priv) != 0)
+		return -EINVAL;
+
+	if (fe->ops.i2c_gate_ctrl)
+		fe->ops.i2c_gate_ctrl(fe, 0);
+
+	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops,
+	       sizeof(struct dvb_tuner_ops));
+	fe->tuner_priv = priv;
+	i2c_set_clientdata(client, priv);
+
+	dev_info(dev, "Sony HELENE attached on addr=%x at I2C adapter %p\n",
+		 priv->i2c_address, priv->i2c);
+
+	return 0;
+}
+
+static const struct i2c_device_id helene_id[] = {
+	{ "helene", },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, helene_id);
+
+static struct i2c_driver helene_driver = {
+	.driver = {
+		.name = "helene",
+	},
+	.probe    = helene_probe,
+	.id_table = helene_id,
+};
+module_i2c_driver(helene_driver);
+
 MODULE_DESCRIPTION("Sony HELENE Sat/Ter tuner driver");
 MODULE_AUTHOR("Abylay Ospan <aospan@netup.ru>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/helene.h b/drivers/media/dvb-frontends/helene.h
index c9fc81c7e4e7..8562d01bc93e 100644
--- a/drivers/media/dvb-frontends/helene.h
+++ b/drivers/media/dvb-frontends/helene.h
@@ -39,6 +39,7 @@ enum helene_xtal {
  * @set_tuner_callback:	Callback function that notifies the parent driver
  *			which tuner is active now
  * @xtal: Cristal frequency as described by &enum helene_xtal
+ * @fe: Frontend for which connects this tuner
  */
 struct helene_config {
 	u8	i2c_address;
@@ -46,6 +47,8 @@ struct helene_config {
 	void	*set_tuner_priv;
 	int	(*set_tuner_callback)(void *, int);
 	enum helene_xtal xtal;
+
+	struct dvb_frontend *fe;
 };
 
 #if IS_REACHABLE(CONFIG_DVB_HELENE)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] media: helene: add I2C device probe function
  2018-07-02  5:12 ` Katsuhiro Suzuki
@ 2018-07-04 17:30   ` Mauro Carvalho Chehab
  -1 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2018-07-04 17:30 UTC (permalink / raw)
  To: Katsuhiro Suzuki
  Cc: linux-media, Sumit Semwal, Masami Hiramatsu, Jassi Brar,
	linux-arm-kernel, linux-kernel

Hi Katsushiro-san,

Em Mon,  2 Jul 2018 14:12:11 +0900
Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com> escreveu:

> This patch adds I2C probe function to use dvb_module_probe()
> with this driver.
> 
> Signed-off-by: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>
> 
> ---
> 
> Changes since v3:
>   - Drop wrong patch 2/2 from v3 series, no changes in this patch
> 
> Changes since v2:
>   - Nothing
> 
> Changes since v1:
>   - Add documents for dvb_frontend member of helene_config
> ---
>  drivers/media/dvb-frontends/helene.c | 88 ++++++++++++++++++++++++++--
>  drivers/media/dvb-frontends/helene.h |  3 +
>  2 files changed, 87 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
> index a0d0b53c91d7..04033f0c278b 100644
> --- a/drivers/media/dvb-frontends/helene.c
> +++ b/drivers/media/dvb-frontends/helene.c
> @@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
>  	return 0;
>  }
>  
> -static int helene_set_params(struct dvb_frontend *fe)
> +static int helene_set_params_t(struct dvb_frontend *fe)
>  {
>  	u8 data[MAX_WRITE_REGSIZE];
>  	u32 frequency;
> @@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
>  	return 0;
>  }
>  
> +static int helene_set_params(struct dvb_frontend *fe)
> +{
> +	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
> +
> +	if (p->delivery_system == SYS_DVBT ||
> +	    p->delivery_system == SYS_DVBT2 ||
> +	    p->delivery_system == SYS_ISDBT ||
> +	    p->delivery_system == SYS_DVBC_ANNEX_A)
> +		return helene_set_params_t(fe);
> +
> +	return helene_set_params_s(fe);
> +}
> +
>  static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
>  {
>  	struct helene_priv *priv = fe->tuner_priv;
> @@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
>  	return 0;
>  }
>  
> -static const struct dvb_tuner_ops helene_tuner_ops = {
> +static const struct dvb_tuner_ops helene_tuner_ops_t = {
>  	.info = {
>  		.name = "Sony HELENE Ter tuner",
>  		.frequency_min = 1000000,
> @@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
>  	.init = helene_init,
>  	.release = helene_release,
>  	.sleep = helene_sleep,
> -	.set_params = helene_set_params,
> +	.set_params = helene_set_params_t,
>  	.get_frequency = helene_get_frequency,
>  };
>  
> @@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
>  	.get_frequency = helene_get_frequency,
>  };
>  
> +static const struct dvb_tuner_ops helene_tuner_ops = {
> +	.info = {
> +		.name = "Sony HELENE Sat/Ter tuner",
> +		.frequency_min = 500000,
> +		.frequency_max = 1200000000,
> +		.frequency_step = 1000,
> +	},
> +	.init = helene_init,
> +	.release = helene_release,
> +	.sleep = helene_sleep,
> +	.set_params = helene_set_params,
> +	.get_frequency = helene_get_frequency,
> +};

About the same issue as I commented on your driver: this is
wrong.

See, for ISDB-T, frequencies are interpreted in Hz. The above
says that this device would be able to range frequencies
between 500 kHz to 1.2 MHz. I doubt that this silicon would be
a frequency of 500 kHz!

For ISDB-S, that would mean a range from 500 MHz to 1.2 GHz.
Again, that's wrong.

The frequency step will also be interpreted wrong (either
for ISDB-T or ISDB-S).

I see two possible fixes for it:

1) Consider all frequencies in Hz for the .info field. That will
require touching all satellite drivers to multiply frequencies
by 1000. It will also require fixing a DVBv3 backward code for
FE_GET_INFO, as userspace expect those values in kHZ.

The change at the core should be simple, but it will require touching
all satellite drivers, in order to consider the frequency in Hz.

Also, it will be hacky, as it will be abusing the DVBv3 uAPI
struct.

2) Apply the enclosed patch at the headers, and adjust everything to work
with frequencies in Hz.

The changes will be large, as all frontend drivers will require changes,
but it will provide a better long term solution.

Perhaps we could do a mid-term approach, adding the new fields in
order to be used by new drivers.

Regards,
Mauro

diff --git a/include/media/dvb_frontend.h b/include/media/dvb_frontend.h
index 331c8269c00e..6c5b919552db 100644
--- a/include/media/dvb_frontend.h
+++ b/include/media/dvb_frontend.h
@@ -73,22 +73,19 @@ struct dvb_frontend;
  * struct dvb_tuner_info - Frontend name and min/max ranges/bandwidths
  *
  * @name:		name of the Frontend
- * @frequency_min:	minimal frequency supported
- * @frequency_max:	maximum frequency supported
- * @frequency_step:	frequency step
+ * @frequency_min_hz:	minimal frequency supported in Hz
+ * @frequency_max_hz:	maximum frequency supported in Hz
+ * @frequency_step_hz:	frequency step in Hz
  * @bandwidth_min:	minimal frontend bandwidth supported
  * @bandwidth_max:	maximum frontend bandwidth supported
  * @bandwidth_step:	frontend bandwidth step
- *
- * NOTE: frequency parameters are in Hz, for terrestrial/cable or kHz for
- * satellite.
  */
 struct dvb_tuner_info {
 	char name[128];
 
-	u32 frequency_min;
-	u32 frequency_max;
-	u32 frequency_step;
+	u32 frequency_min_hz;
+	u32 frequency_max_hz;
+	u32 frequency_step_hz;
 
 	u32 bandwidth_min;
 	u32 bandwidth_max;
@@ -403,7 +400,15 @@ struct dtv_frontend_properties;
  * @analog_ops:		pointer to &struct analog_demod_ops
  */
 struct dvb_frontend_ops {
-	struct dvb_frontend_info info;
+	char       name[128];
+	__u32      frequency_min_hz;
+	__u32      frequency_max_hz;
+	__u32      frequency_stepsize_hz;
+	__u32      frequency_tolerance_hz;
+	__u32      symbol_rate_min;
+	__u32      symbol_rate_max;
+	__u32      symbol_rate_tolerance;
+	enum fe_caps caps;
 
 	u8 delsys[MAX_DELSYS];

Thanks,
Mauro

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v4] media: helene: add I2C device probe function
@ 2018-07-04 17:30   ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2018-07-04 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Katsushiro-san,

Em Mon,  2 Jul 2018 14:12:11 +0900
Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com> escreveu:

> This patch adds I2C probe function to use dvb_module_probe()
> with this driver.
> 
> Signed-off-by: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>
> 
> ---
> 
> Changes since v3:
>   - Drop wrong patch 2/2 from v3 series, no changes in this patch
> 
> Changes since v2:
>   - Nothing
> 
> Changes since v1:
>   - Add documents for dvb_frontend member of helene_config
> ---
>  drivers/media/dvb-frontends/helene.c | 88 ++++++++++++++++++++++++++--
>  drivers/media/dvb-frontends/helene.h |  3 +
>  2 files changed, 87 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
> index a0d0b53c91d7..04033f0c278b 100644
> --- a/drivers/media/dvb-frontends/helene.c
> +++ b/drivers/media/dvb-frontends/helene.c
> @@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
>  	return 0;
>  }
>  
> -static int helene_set_params(struct dvb_frontend *fe)
> +static int helene_set_params_t(struct dvb_frontend *fe)
>  {
>  	u8 data[MAX_WRITE_REGSIZE];
>  	u32 frequency;
> @@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
>  	return 0;
>  }
>  
> +static int helene_set_params(struct dvb_frontend *fe)
> +{
> +	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
> +
> +	if (p->delivery_system == SYS_DVBT ||
> +	    p->delivery_system == SYS_DVBT2 ||
> +	    p->delivery_system == SYS_ISDBT ||
> +	    p->delivery_system == SYS_DVBC_ANNEX_A)
> +		return helene_set_params_t(fe);
> +
> +	return helene_set_params_s(fe);
> +}
> +
>  static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
>  {
>  	struct helene_priv *priv = fe->tuner_priv;
> @@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
>  	return 0;
>  }
>  
> -static const struct dvb_tuner_ops helene_tuner_ops = {
> +static const struct dvb_tuner_ops helene_tuner_ops_t = {
>  	.info = {
>  		.name = "Sony HELENE Ter tuner",
>  		.frequency_min = 1000000,
> @@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
>  	.init = helene_init,
>  	.release = helene_release,
>  	.sleep = helene_sleep,
> -	.set_params = helene_set_params,
> +	.set_params = helene_set_params_t,
>  	.get_frequency = helene_get_frequency,
>  };
>  
> @@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
>  	.get_frequency = helene_get_frequency,
>  };
>  
> +static const struct dvb_tuner_ops helene_tuner_ops = {
> +	.info = {
> +		.name = "Sony HELENE Sat/Ter tuner",
> +		.frequency_min = 500000,
> +		.frequency_max = 1200000000,
> +		.frequency_step = 1000,
> +	},
> +	.init = helene_init,
> +	.release = helene_release,
> +	.sleep = helene_sleep,
> +	.set_params = helene_set_params,
> +	.get_frequency = helene_get_frequency,
> +};

About the same issue as I commented on your driver: this is
wrong.

See, for ISDB-T, frequencies are interpreted in Hz. The above
says that this device would be able to range frequencies
between 500 kHz to 1.2 MHz. I doubt that this silicon would be
a frequency of 500 kHz!

For ISDB-S, that would mean a range from 500 MHz to 1.2 GHz.
Again, that's wrong.

The frequency step will also be interpreted wrong (either
for ISDB-T or ISDB-S).

I see two possible fixes for it:

1) Consider all frequencies in Hz for the .info field. That will
require touching all satellite drivers to multiply frequencies
by 1000. It will also require fixing a DVBv3 backward code for
FE_GET_INFO, as userspace expect those values in kHZ.

The change at the core should be simple, but it will require touching
all satellite drivers, in order to consider the frequency in Hz.

Also, it will be hacky, as it will be abusing the DVBv3 uAPI
struct.

2) Apply the enclosed patch at the headers, and adjust everything to work
with frequencies in Hz.

The changes will be large, as all frontend drivers will require changes,
but it will provide a better long term solution.

Perhaps we could do a mid-term approach, adding the new fields in
order to be used by new drivers.

Regards,
Mauro

diff --git a/include/media/dvb_frontend.h b/include/media/dvb_frontend.h
index 331c8269c00e..6c5b919552db 100644
--- a/include/media/dvb_frontend.h
+++ b/include/media/dvb_frontend.h
@@ -73,22 +73,19 @@ struct dvb_frontend;
  * struct dvb_tuner_info - Frontend name and min/max ranges/bandwidths
  *
  * @name:		name of the Frontend
- * @frequency_min:	minimal frequency supported
- * @frequency_max:	maximum frequency supported
- * @frequency_step:	frequency step
+ * @frequency_min_hz:	minimal frequency supported in Hz
+ * @frequency_max_hz:	maximum frequency supported in Hz
+ * @frequency_step_hz:	frequency step in Hz
  * @bandwidth_min:	minimal frontend bandwidth supported
  * @bandwidth_max:	maximum frontend bandwidth supported
  * @bandwidth_step:	frontend bandwidth step
- *
- * NOTE: frequency parameters are in Hz, for terrestrial/cable or kHz for
- * satellite.
  */
 struct dvb_tuner_info {
 	char name[128];
 
-	u32 frequency_min;
-	u32 frequency_max;
-	u32 frequency_step;
+	u32 frequency_min_hz;
+	u32 frequency_max_hz;
+	u32 frequency_step_hz;
 
 	u32 bandwidth_min;
 	u32 bandwidth_max;
@@ -403,7 +400,15 @@ struct dtv_frontend_properties;
  * @analog_ops:		pointer to &struct analog_demod_ops
  */
 struct dvb_frontend_ops {
-	struct dvb_frontend_info info;
+	char       name[128];
+	__u32      frequency_min_hz;
+	__u32      frequency_max_hz;
+	__u32      frequency_stepsize_hz;
+	__u32      frequency_tolerance_hz;
+	__u32      symbol_rate_min;
+	__u32      symbol_rate_max;
+	__u32      symbol_rate_tolerance;
+	enum fe_caps caps;
 
 	u8 delsys[MAX_DELSYS];

Thanks,
Mauro

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-07-04 17:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-02  5:12 [PATCH v4] media: helene: add I2C device probe function Katsuhiro Suzuki
2018-07-02  5:12 ` Katsuhiro Suzuki
2018-07-04 17:30 ` Mauro Carvalho Chehab
2018-07-04 17:30   ` Mauro Carvalho Chehab

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.