Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-06-26 Thread Takiguchi, Yasunari


On 2017/06/26 10:24, Mauro Carvalho Chehab wrote:
> Em Sun, 25 Jun 2017 09:15:06 -0300
> Mauro Carvalho Chehab  escreveu:
> 
>> Em Fri, 23 Jun 2017 10:02:39 -0300
>> Mauro Carvalho Chehab  escreveu:
>>
>>> Em Mon, 19 Jun 2017 16:56:13 +0900
>>> "Takiguchi, Yasunari"  escreveu:
>>>   
>> +static int cxd2880_get_frontend_t(struct dvb_frontend *fe,
>> +struct dtv_frontend_properties *c)
>> +{
>> +enum cxd2880_ret ret = CXD2880_RESULT_OK;
>> +int result = 0;
>> +struct cxd2880_priv *priv = NULL;
>> +enum cxd2880_dvbt_mode mode = CXD2880_DVBT_MODE_2K;
>> +enum cxd2880_dvbt_guard guard = CXD2880_DVBT_GUARD_1_32;
>> +struct cxd2880_dvbt_tpsinfo tps;
>> +enum cxd2880_tnrdmd_spectrum_sense sense;
>> +u16 snr = 0;
>> +int strength = 0;
>> +u32 pre_bit_err = 0, pre_bit_count = 0;
>> +u32 post_bit_err = 0, post_bit_count = 0;
>> +u32 block_err = 0, block_count = 0;
>> +
>> +if ((!fe) || (!c)) {
>> +pr_err("%s: invalid arg\n", __func__);
>> +return -EINVAL;
>> +}
>> +
>> +priv = (struct cxd2880_priv *)fe->demodulator_priv;
>> +
>> +mutex_lock(priv->spi_mutex);
>> +ret = cxd2880_tnrdmd_dvbt_mon_mode_guard(>tnrdmd,
>> + , );
>> +mutex_unlock(priv->spi_mutex);
>> +if (ret == CXD2880_RESULT_OK) {
>> +switch (mode) {
>> +case CXD2880_DVBT_MODE_2K:
>> +c->transmission_mode = TRANSMISSION_MODE_2K;
>> +break;
>> +case CXD2880_DVBT_MODE_8K:
>> +c->transmission_mode = TRANSMISSION_MODE_8K;
>> +break;
>> +default:
>> +c->transmission_mode = TRANSMISSION_MODE_2K;
>> +dev_err(>spi->dev, "%s: get invalid mode 
>> %d\n",
>> +__func__, mode);
>> +break;
>> +}
>> +switch (guard) {
>> +case CXD2880_DVBT_GUARD_1_32:
>> +c->guard_interval = GUARD_INTERVAL_1_32;
>> +break;
>> +case CXD2880_DVBT_GUARD_1_16:
>> +c->guard_interval = GUARD_INTERVAL_1_16;
>> +break;
>> +case CXD2880_DVBT_GUARD_1_8:
>> +c->guard_interval = GUARD_INTERVAL_1_8;
>> +break;
>> +case CXD2880_DVBT_GUARD_1_4:
>> +c->guard_interval = GUARD_INTERVAL_1_4;
>> +break;
>> +default:
>> +c->guard_interval = GUARD_INTERVAL_1_32;
>> +dev_err(>spi->dev, "%s: get invalid guard 
>> %d\n",
>> +__func__, guard);
>> +break;
>> +}
>> +} else {
>> +c->transmission_mode = TRANSMISSION_MODE_2K;
>> +c->guard_interval = GUARD_INTERVAL_1_32;
>> +dev_dbg(>spi->dev,
>> +"%s: ModeGuard err %d\n", __func__, ret);
>> +}
>> +
>> +mutex_lock(priv->spi_mutex);
>> +ret = cxd2880_tnrdmd_dvbt_mon_tps_info(>tnrdmd, );
>> +mutex_unlock(priv->spi_mutex);
>> +if (ret == CXD2880_RESULT_OK) {
>> +switch (tps.hierarchy) {
>> +case CXD2880_DVBT_HIERARCHY_NON:
>> +c->hierarchy = HIERARCHY_NONE;
>> +break;
>> +case CXD2880_DVBT_HIERARCHY_1:
>> +c->hierarchy = HIERARCHY_1;
>> +break;
>> +case CXD2880_DVBT_HIERARCHY_2:
>> +c->hierarchy = HIERARCHY_2;
>> +break;
>> +case CXD2880_DVBT_HIERARCHY_4:
>> +c->hierarchy = HIERARCHY_4;
>> +break;
>> +default:
>> +c->hierarchy = HIERARCHY_NONE;
>> +dev_err(>spi->dev,
>> +"%s: TPSInfo hierarchy invalid %d\n",
>> +__func__, tps.hierarchy);
>> +break;
>> +}
>> +
>> +switch (tps.rate_hp) {
>> +

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-06-25 Thread Mauro Carvalho Chehab
Em Sun, 25 Jun 2017 09:15:06 -0300
Mauro Carvalho Chehab  escreveu:

> Em Fri, 23 Jun 2017 10:02:39 -0300
> Mauro Carvalho Chehab  escreveu:
> 
> > Em Mon, 19 Jun 2017 16:56:13 +0900
> > "Takiguchi, Yasunari"  escreveu:
> >   
> > > >> +static int cxd2880_get_frontend_t(struct dvb_frontend *fe,
> > > >> +  struct dtv_frontend_properties *c)
> > > >> +{
> > > >> +  enum cxd2880_ret ret = CXD2880_RESULT_OK;
> > > >> +  int result = 0;
> > > >> +  struct cxd2880_priv *priv = NULL;
> > > >> +  enum cxd2880_dvbt_mode mode = CXD2880_DVBT_MODE_2K;
> > > >> +  enum cxd2880_dvbt_guard guard = CXD2880_DVBT_GUARD_1_32;
> > > >> +  struct cxd2880_dvbt_tpsinfo tps;
> > > >> +  enum cxd2880_tnrdmd_spectrum_sense sense;
> > > >> +  u16 snr = 0;
> > > >> +  int strength = 0;
> > > >> +  u32 pre_bit_err = 0, pre_bit_count = 0;
> > > >> +  u32 post_bit_err = 0, post_bit_count = 0;
> > > >> +  u32 block_err = 0, block_count = 0;
> > > >> +
> > > >> +  if ((!fe) || (!c)) {
> > > >> +  pr_err("%s: invalid arg\n", __func__);
> > > >> +  return -EINVAL;
> > > >> +  }
> > > >> +
> > > >> +  priv = (struct cxd2880_priv *)fe->demodulator_priv;
> > > >> +
> > > >> +  mutex_lock(priv->spi_mutex);
> > > >> +  ret = cxd2880_tnrdmd_dvbt_mon_mode_guard(>tnrdmd,
> > > >> +   , );
> > > >> +  mutex_unlock(priv->spi_mutex);
> > > >> +  if (ret == CXD2880_RESULT_OK) {
> > > >> +  switch (mode) {
> > > >> +  case CXD2880_DVBT_MODE_2K:
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_MODE_8K:
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_8K;
> > > >> +  break;
> > > >> +  default:
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> > > >> +  dev_err(>spi->dev, "%s: get invalid mode 
> > > >> %d\n",
> > > >> +  __func__, mode);
> > > >> +  break;
> > > >> +  }
> > > >> +  switch (guard) {
> > > >> +  case CXD2880_DVBT_GUARD_1_32:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_GUARD_1_16:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_16;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_GUARD_1_8:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_8;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_GUARD_1_4:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_4;
> > > >> +  break;
> > > >> +  default:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> > > >> +  dev_err(>spi->dev, "%s: get invalid guard 
> > > >> %d\n",
> > > >> +  __func__, guard);
> > > >> +  break;
> > > >> +  }
> > > >> +  } else {
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> > > >> +  dev_dbg(>spi->dev,
> > > >> +  "%s: ModeGuard err %d\n", __func__, ret);
> > > >> +  }
> > > >> +
> > > >> +  mutex_lock(priv->spi_mutex);
> > > >> +  ret = cxd2880_tnrdmd_dvbt_mon_tps_info(>tnrdmd, );
> > > >> +  mutex_unlock(priv->spi_mutex);
> > > >> +  if (ret == CXD2880_RESULT_OK) {
> > > >> +  switch (tps.hierarchy) {
> > > >> +  case CXD2880_DVBT_HIERARCHY_NON:
> > > >> +  c->hierarchy = HIERARCHY_NONE;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_HIERARCHY_1:
> > > >> +  c->hierarchy = HIERARCHY_1;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_HIERARCHY_2:
> > > >> +  c->hierarchy = HIERARCHY_2;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_HIERARCHY_4:
> > > >> +  c->hierarchy = HIERARCHY_4;
> > > >> +  break;
> > > >> +  default:
> > > >> +  c->hierarchy = HIERARCHY_NONE;
> > > >> +  dev_err(>spi->dev,
> > > >> +  "%s: TPSInfo hierarchy invalid %d\n",
> > > >> +  __func__, tps.hierarchy);
> > > >> +  break;
> > > >> +  }
> > > >> +
> > > >> +  switch (tps.rate_hp) {
> > > >> +  case CXD2880_DVBT_CODERATE_1_2:
> > > 

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-06-25 Thread Mauro Carvalho Chehab
Em Fri, 23 Jun 2017 10:02:39 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 19 Jun 2017 16:56:13 +0900
> "Takiguchi, Yasunari"  escreveu:
> 
> > >> +static int cxd2880_get_frontend_t(struct dvb_frontend *fe,
> > >> +struct dtv_frontend_properties *c)
> > >> +{
> > >> +enum cxd2880_ret ret = CXD2880_RESULT_OK;
> > >> +int result = 0;
> > >> +struct cxd2880_priv *priv = NULL;
> > >> +enum cxd2880_dvbt_mode mode = CXD2880_DVBT_MODE_2K;
> > >> +enum cxd2880_dvbt_guard guard = CXD2880_DVBT_GUARD_1_32;
> > >> +struct cxd2880_dvbt_tpsinfo tps;
> > >> +enum cxd2880_tnrdmd_spectrum_sense sense;
> > >> +u16 snr = 0;
> > >> +int strength = 0;
> > >> +u32 pre_bit_err = 0, pre_bit_count = 0;
> > >> +u32 post_bit_err = 0, post_bit_count = 0;
> > >> +u32 block_err = 0, block_count = 0;
> > >> +
> > >> +if ((!fe) || (!c)) {
> > >> +pr_err("%s: invalid arg\n", __func__);
> > >> +return -EINVAL;
> > >> +}
> > >> +
> > >> +priv = (struct cxd2880_priv *)fe->demodulator_priv;
> > >> +
> > >> +mutex_lock(priv->spi_mutex);
> > >> +ret = cxd2880_tnrdmd_dvbt_mon_mode_guard(>tnrdmd,
> > >> + , );
> > >> +mutex_unlock(priv->spi_mutex);
> > >> +if (ret == CXD2880_RESULT_OK) {
> > >> +switch (mode) {
> > >> +case CXD2880_DVBT_MODE_2K:
> > >> +c->transmission_mode = TRANSMISSION_MODE_2K;
> > >> +break;
> > >> +case CXD2880_DVBT_MODE_8K:
> > >> +c->transmission_mode = TRANSMISSION_MODE_8K;
> > >> +break;
> > >> +default:
> > >> +c->transmission_mode = TRANSMISSION_MODE_2K;
> > >> +dev_err(>spi->dev, "%s: get invalid mode 
> > >> %d\n",
> > >> +__func__, mode);
> > >> +break;
> > >> +}
> > >> +switch (guard) {
> > >> +case CXD2880_DVBT_GUARD_1_32:
> > >> +c->guard_interval = GUARD_INTERVAL_1_32;
> > >> +break;
> > >> +case CXD2880_DVBT_GUARD_1_16:
> > >> +c->guard_interval = GUARD_INTERVAL_1_16;
> > >> +break;
> > >> +case CXD2880_DVBT_GUARD_1_8:
> > >> +c->guard_interval = GUARD_INTERVAL_1_8;
> > >> +break;
> > >> +case CXD2880_DVBT_GUARD_1_4:
> > >> +c->guard_interval = GUARD_INTERVAL_1_4;
> > >> +break;
> > >> +default:
> > >> +c->guard_interval = GUARD_INTERVAL_1_32;
> > >> +dev_err(>spi->dev, "%s: get invalid guard 
> > >> %d\n",
> > >> +__func__, guard);
> > >> +break;
> > >> +}
> > >> +} else {
> > >> +c->transmission_mode = TRANSMISSION_MODE_2K;
> > >> +c->guard_interval = GUARD_INTERVAL_1_32;
> > >> +dev_dbg(>spi->dev,
> > >> +"%s: ModeGuard err %d\n", __func__, ret);
> > >> +}
> > >> +
> > >> +mutex_lock(priv->spi_mutex);
> > >> +ret = cxd2880_tnrdmd_dvbt_mon_tps_info(>tnrdmd, );
> > >> +mutex_unlock(priv->spi_mutex);
> > >> +if (ret == CXD2880_RESULT_OK) {
> > >> +switch (tps.hierarchy) {
> > >> +case CXD2880_DVBT_HIERARCHY_NON:
> > >> +c->hierarchy = HIERARCHY_NONE;
> > >> +break;
> > >> +case CXD2880_DVBT_HIERARCHY_1:
> > >> +c->hierarchy = HIERARCHY_1;
> > >> +break;
> > >> +case CXD2880_DVBT_HIERARCHY_2:
> > >> +c->hierarchy = HIERARCHY_2;
> > >> +break;
> > >> +case CXD2880_DVBT_HIERARCHY_4:
> > >> +c->hierarchy = HIERARCHY_4;
> > >> +break;
> > >> +default:
> > >> +c->hierarchy = HIERARCHY_NONE;
> > >> +dev_err(>spi->dev,
> > >> +"%s: TPSInfo hierarchy invalid %d\n",
> > >> +__func__, tps.hierarchy);
> > >> +break;
> > >> +}
> > >> +
> > >> +switch (tps.rate_hp) {
> > >> +case CXD2880_DVBT_CODERATE_1_2:
> > >> +c->code_rate_HP = FEC_1_2;
> > >> +break;
> > >> +case 

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-06-23 Thread Mauro Carvalho Chehab
Em Mon, 19 Jun 2017 16:56:13 +0900
"Takiguchi, Yasunari"  escreveu:

> >> +static int cxd2880_get_frontend_t(struct dvb_frontend *fe,
> >> +  struct dtv_frontend_properties *c)
> >> +{
> >> +  enum cxd2880_ret ret = CXD2880_RESULT_OK;
> >> +  int result = 0;
> >> +  struct cxd2880_priv *priv = NULL;
> >> +  enum cxd2880_dvbt_mode mode = CXD2880_DVBT_MODE_2K;
> >> +  enum cxd2880_dvbt_guard guard = CXD2880_DVBT_GUARD_1_32;
> >> +  struct cxd2880_dvbt_tpsinfo tps;
> >> +  enum cxd2880_tnrdmd_spectrum_sense sense;
> >> +  u16 snr = 0;
> >> +  int strength = 0;
> >> +  u32 pre_bit_err = 0, pre_bit_count = 0;
> >> +  u32 post_bit_err = 0, post_bit_count = 0;
> >> +  u32 block_err = 0, block_count = 0;
> >> +
> >> +  if ((!fe) || (!c)) {
> >> +  pr_err("%s: invalid arg\n", __func__);
> >> +  return -EINVAL;
> >> +  }
> >> +
> >> +  priv = (struct cxd2880_priv *)fe->demodulator_priv;
> >> +
> >> +  mutex_lock(priv->spi_mutex);
> >> +  ret = cxd2880_tnrdmd_dvbt_mon_mode_guard(>tnrdmd,
> >> +   , );
> >> +  mutex_unlock(priv->spi_mutex);
> >> +  if (ret == CXD2880_RESULT_OK) {
> >> +  switch (mode) {
> >> +  case CXD2880_DVBT_MODE_2K:
> >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> >> +  break;
> >> +  case CXD2880_DVBT_MODE_8K:
> >> +  c->transmission_mode = TRANSMISSION_MODE_8K;
> >> +  break;
> >> +  default:
> >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> >> +  dev_err(>spi->dev, "%s: get invalid mode %d\n",
> >> +  __func__, mode);
> >> +  break;
> >> +  }
> >> +  switch (guard) {
> >> +  case CXD2880_DVBT_GUARD_1_32:
> >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> >> +  break;
> >> +  case CXD2880_DVBT_GUARD_1_16:
> >> +  c->guard_interval = GUARD_INTERVAL_1_16;
> >> +  break;
> >> +  case CXD2880_DVBT_GUARD_1_8:
> >> +  c->guard_interval = GUARD_INTERVAL_1_8;
> >> +  break;
> >> +  case CXD2880_DVBT_GUARD_1_4:
> >> +  c->guard_interval = GUARD_INTERVAL_1_4;
> >> +  break;
> >> +  default:
> >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> >> +  dev_err(>spi->dev, "%s: get invalid guard %d\n",
> >> +  __func__, guard);
> >> +  break;
> >> +  }
> >> +  } else {
> >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> >> +  dev_dbg(>spi->dev,
> >> +  "%s: ModeGuard err %d\n", __func__, ret);
> >> +  }
> >> +
> >> +  mutex_lock(priv->spi_mutex);
> >> +  ret = cxd2880_tnrdmd_dvbt_mon_tps_info(>tnrdmd, );
> >> +  mutex_unlock(priv->spi_mutex);
> >> +  if (ret == CXD2880_RESULT_OK) {
> >> +  switch (tps.hierarchy) {
> >> +  case CXD2880_DVBT_HIERARCHY_NON:
> >> +  c->hierarchy = HIERARCHY_NONE;
> >> +  break;
> >> +  case CXD2880_DVBT_HIERARCHY_1:
> >> +  c->hierarchy = HIERARCHY_1;
> >> +  break;
> >> +  case CXD2880_DVBT_HIERARCHY_2:
> >> +  c->hierarchy = HIERARCHY_2;
> >> +  break;
> >> +  case CXD2880_DVBT_HIERARCHY_4:
> >> +  c->hierarchy = HIERARCHY_4;
> >> +  break;
> >> +  default:
> >> +  c->hierarchy = HIERARCHY_NONE;
> >> +  dev_err(>spi->dev,
> >> +  "%s: TPSInfo hierarchy invalid %d\n",
> >> +  __func__, tps.hierarchy);
> >> +  break;
> >> +  }
> >> +
> >> +  switch (tps.rate_hp) {
> >> +  case CXD2880_DVBT_CODERATE_1_2:
> >> +  c->code_rate_HP = FEC_1_2;
> >> +  break;
> >> +  case CXD2880_DVBT_CODERATE_2_3:
> >> +  c->code_rate_HP = FEC_2_3;
> >> +  break;
> >> +  case CXD2880_DVBT_CODERATE_3_4:
> >> +  c->code_rate_HP = FEC_3_4;
> >> +  break;
> >> +  case CXD2880_DVBT_CODERATE_5_6:
> >> +  c->code_rate_HP = FEC_5_6;
> >> +  break;
> >> +  case CXD2880_DVBT_CODERATE_7_8:
> >> +  c->code_rate_HP = FEC_7_8;
> >> +  break;
> >> +  default:
> >> +  c->code_rate_HP = FEC_NONE;
> >> +  dev_err(>spi->dev,
> >> +  "%s: TPSInfo rateHP invalid %d\n",
> >> +  __func__, tps.rate_hp);
> >> +  break;
> >> +  }
> >> +  switch (tps.rate_lp) {
> >> +  case 

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-06-19 Thread Takiguchi, Yasunari
Thanks for your kindly reply.

We are think of how to change our driver to follow the comments now.
I reply one thing about reading BER measures at get_frontend() in this mail.

On 2017/06/13 22:30, Mauro Carvalho Chehab wrote:
> Em Fri, 14 Apr 2017 11:31:50 +0900
>  escreveu:
> 
>> From: Yasunari Takiguchi 
>>
>> This provides the main dvb frontend operation functions
>> for the Sony CXD2880 DVB-T2/T tuner + demodulator driver.
> 
> For now, I'll do only a quick review on patches 6-15, as there are several
> things to be ajusted on the code, from my past comments.
> 
> I'll do a better review on the next version. So, I'll focus only on
> a few random issues I see on the code below.
> 
>>
>> Signed-off-by: Yasunari Takiguchi 
>> Signed-off-by: Masayuki Yamamoto 
>> Signed-off-by: Hideki Nozawa 
>> Signed-off-by: Kota Yonezawa 
>> Signed-off-by: Toshihiko Matsumoto 
>> Signed-off-by: Satoshi Watanabe 
>> ---
>>  drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 1550 
>> +
>>  1 file changed, 1550 insertions(+)
>>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>>
>> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c 
>> b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>> new file mode 100644
>> index ..66d78fb93a13
>> --- /dev/null
>> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>> @@ -0,0 +1,1550 @@
>> +/*
>> + * cxd2880_top.c
>> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
>> + *
>> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; version 2 of the License.
>> + *
>> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
>> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
>> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
>> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
>> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see .
>> + */
>> +
>> +#include 
>> +
>> +#include "dvb_frontend.h"
>> +
>> +#include "cxd2880.h"
>> +#include "cxd2880_tnrdmd_mon.h"
>> +#include "cxd2880_tnrdmd_dvbt2_mon.h"
>> +#include "cxd2880_tnrdmd_dvbt_mon.h"
>> +#include "cxd2880_integ_dvbt2.h"
>> +#include "cxd2880_integ_dvbt.h"
>> +#include "cxd2880_devio_spi.h"
>> +#include "cxd2880_spi_device.h"
>> +#include "cxd2880_tnrdmd_driver_version.h"
>> +
>> +struct cxd2880_priv {
>> +struct cxd2880_tnrdmd tnrdmd;
>> +struct spi_device *spi;
>> +struct cxd2880_io regio;
>> +struct cxd2880_spi_device spi_device;
>> +struct cxd2880_spi cxd2880_spi;
>> +struct cxd2880_dvbt_tune_param dvbt_tune_param;
>> +struct cxd2880_dvbt2_tune_param dvbt2_tune_param;
>> +struct mutex *spi_mutex; /* For SPI access exclusive control */
>> +};
>> +
>> +/*
>> + * return value conversion table
>> + */
>> +static int return_tbl[] = {
>> +0, /* CXD2880_RESULT_OK */
>> +-EINVAL,   /* CXD2880_RESULT_ERROR_ARG*/
>> +-EIO,  /* CXD2880_RESULT_ERROR_IO */
>> +-EPERM,/* CXD2880_RESULT_ERROR_SW_STATE */
>> +-EBUSY,/* CXD2880_RESULT_ERROR_HW_STATE */
>> +-ETIME,/* CXD2880_RESULT_ERROR_TIMEOUT */
>> +-EAGAIN,   /* CXD2880_RESULT_ERROR_UNLOCK */
>> +-ERANGE,   /* CXD2880_RESULT_ERROR_RANGE */
>> +-EOPNOTSUPP,   /* CXD2880_RESULT_ERROR_NOSUPPORT */
>> +-ECANCELED,/* CXD2880_RESULT_ERROR_CANCEL */
>> +-EPERM,/* CXD2880_RESULT_ERROR_OTHER */
>> +-EOVERFLOW,/* CXD2880_RESULT_ERROR_OVERFLOW */
>> +0, /* CXD2880_RESULT_OK_CONFIRM */
>> +};
> 
> Please, don't use a conversion table. That makes the driver more
> complex to be understood without any good reason. Instead, just 
> replace the values at the original enum.
> 
>> +
>> +static enum cxd2880_ret cxd2880_pre_bit_err_t(
>> +struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
>> +u32 *pre_bit_count)
>> +{
>> +u8 rdata[2];
>> +
>> +if ((!tnrdmd) || (!pre_bit_err) || 

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-06-13 Thread Mauro Carvalho Chehab
Em Fri, 14 Apr 2017 11:31:50 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> This provides the main dvb frontend operation functions
> for the Sony CXD2880 DVB-T2/T tuner + demodulator driver.

For now, I'll do only a quick review on patches 6-15, as there are several
things to be ajusted on the code, from my past comments.

I'll do a better review on the next version. So, I'll focus only on
a few random issues I see on the code below.

> 
> Signed-off-by: Yasunari Takiguchi 
> Signed-off-by: Masayuki Yamamoto 
> Signed-off-by: Hideki Nozawa 
> Signed-off-by: Kota Yonezawa 
> Signed-off-by: Toshihiko Matsumoto 
> Signed-off-by: Satoshi Watanabe 
> ---
>  drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 1550 
> +
>  1 file changed, 1550 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c 
> b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> new file mode 100644
> index ..66d78fb93a13
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> @@ -0,0 +1,1550 @@
> +/*
> + * cxd2880_top.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include 
> +
> +#include "dvb_frontend.h"
> +
> +#include "cxd2880.h"
> +#include "cxd2880_tnrdmd_mon.h"
> +#include "cxd2880_tnrdmd_dvbt2_mon.h"
> +#include "cxd2880_tnrdmd_dvbt_mon.h"
> +#include "cxd2880_integ_dvbt2.h"
> +#include "cxd2880_integ_dvbt.h"
> +#include "cxd2880_devio_spi.h"
> +#include "cxd2880_spi_device.h"
> +#include "cxd2880_tnrdmd_driver_version.h"
> +
> +struct cxd2880_priv {
> + struct cxd2880_tnrdmd tnrdmd;
> + struct spi_device *spi;
> + struct cxd2880_io regio;
> + struct cxd2880_spi_device spi_device;
> + struct cxd2880_spi cxd2880_spi;
> + struct cxd2880_dvbt_tune_param dvbt_tune_param;
> + struct cxd2880_dvbt2_tune_param dvbt2_tune_param;
> + struct mutex *spi_mutex; /* For SPI access exclusive control */
> +};
> +
> +/*
> + * return value conversion table
> + */
> +static int return_tbl[] = {
> + 0, /* CXD2880_RESULT_OK */
> + -EINVAL,   /* CXD2880_RESULT_ERROR_ARG*/
> + -EIO,  /* CXD2880_RESULT_ERROR_IO */
> + -EPERM,/* CXD2880_RESULT_ERROR_SW_STATE */
> + -EBUSY,/* CXD2880_RESULT_ERROR_HW_STATE */
> + -ETIME,/* CXD2880_RESULT_ERROR_TIMEOUT */
> + -EAGAIN,   /* CXD2880_RESULT_ERROR_UNLOCK */
> + -ERANGE,   /* CXD2880_RESULT_ERROR_RANGE */
> + -EOPNOTSUPP,   /* CXD2880_RESULT_ERROR_NOSUPPORT */
> + -ECANCELED,/* CXD2880_RESULT_ERROR_CANCEL */
> + -EPERM,/* CXD2880_RESULT_ERROR_OTHER */
> + -EOVERFLOW,/* CXD2880_RESULT_ERROR_OVERFLOW */
> + 0, /* CXD2880_RESULT_OK_CONFIRM */
> +};

Please, don't use a conversion table. That makes the driver more
complex to be understood without any good reason. Instead, just 
replace the values at the original enum.

> +
> +static enum cxd2880_ret cxd2880_pre_bit_err_t(
> + struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
> + u32 *pre_bit_count)
> +{
> + u8 rdata[2];
> +
> + if ((!tnrdmd) || (!pre_bit_err) || (!pre_bit_count))
> + return CXD2880_RESULT_ERROR_ARG;
> +
> + if (tnrdmd->diver_mode == CXD2880_TNRDMD_DIVERMODE_SUB)
> + return CXD2880_RESULT_ERROR_ARG;
> +
> + if (tnrdmd->state != CXD2880_TNRDMD_STATE_ACTIVE)
> + return CXD2880_RESULT_ERROR_SW_STATE;
> +
> + if (tnrdmd->sys != 

[PATCH v2 08/15] [media] cxd2880: Add top level of the driver

2017-04-13 Thread Yasunari.Takiguchi
From: Yasunari Takiguchi 

This provides the main dvb frontend operation functions
for the Sony CXD2880 DVB-T2/T tuner + demodulator driver.

Signed-off-by: Yasunari Takiguchi 
Signed-off-by: Masayuki Yamamoto 
Signed-off-by: Hideki Nozawa 
Signed-off-by: Kota Yonezawa 
Signed-off-by: Toshihiko Matsumoto 
Signed-off-by: Satoshi Watanabe 
---
 drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 1550 +
 1 file changed, 1550 insertions(+)
 create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_top.c

diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c 
b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
new file mode 100644
index ..66d78fb93a13
--- /dev/null
+++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
@@ -0,0 +1,1550 @@
+/*
+ * cxd2880_top.c
+ * Sony CXD2880 DVB-T2/T tuner + demodulator driver
+ *
+ * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
+ * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include 
+
+#include "dvb_frontend.h"
+
+#include "cxd2880.h"
+#include "cxd2880_tnrdmd_mon.h"
+#include "cxd2880_tnrdmd_dvbt2_mon.h"
+#include "cxd2880_tnrdmd_dvbt_mon.h"
+#include "cxd2880_integ_dvbt2.h"
+#include "cxd2880_integ_dvbt.h"
+#include "cxd2880_devio_spi.h"
+#include "cxd2880_spi_device.h"
+#include "cxd2880_tnrdmd_driver_version.h"
+
+struct cxd2880_priv {
+   struct cxd2880_tnrdmd tnrdmd;
+   struct spi_device *spi;
+   struct cxd2880_io regio;
+   struct cxd2880_spi_device spi_device;
+   struct cxd2880_spi cxd2880_spi;
+   struct cxd2880_dvbt_tune_param dvbt_tune_param;
+   struct cxd2880_dvbt2_tune_param dvbt2_tune_param;
+   struct mutex *spi_mutex; /* For SPI access exclusive control */
+};
+
+/*
+ * return value conversion table
+ */
+static int return_tbl[] = {
+   0, /* CXD2880_RESULT_OK */
+   -EINVAL,   /* CXD2880_RESULT_ERROR_ARG*/
+   -EIO,  /* CXD2880_RESULT_ERROR_IO */
+   -EPERM,/* CXD2880_RESULT_ERROR_SW_STATE */
+   -EBUSY,/* CXD2880_RESULT_ERROR_HW_STATE */
+   -ETIME,/* CXD2880_RESULT_ERROR_TIMEOUT */
+   -EAGAIN,   /* CXD2880_RESULT_ERROR_UNLOCK */
+   -ERANGE,   /* CXD2880_RESULT_ERROR_RANGE */
+   -EOPNOTSUPP,   /* CXD2880_RESULT_ERROR_NOSUPPORT */
+   -ECANCELED,/* CXD2880_RESULT_ERROR_CANCEL */
+   -EPERM,/* CXD2880_RESULT_ERROR_OTHER */
+   -EOVERFLOW,/* CXD2880_RESULT_ERROR_OVERFLOW */
+   0, /* CXD2880_RESULT_OK_CONFIRM */
+};
+
+static enum cxd2880_ret cxd2880_pre_bit_err_t(
+   struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
+   u32 *pre_bit_count)
+{
+   u8 rdata[2];
+
+   if ((!tnrdmd) || (!pre_bit_err) || (!pre_bit_count))
+   return CXD2880_RESULT_ERROR_ARG;
+
+   if (tnrdmd->diver_mode == CXD2880_TNRDMD_DIVERMODE_SUB)
+   return CXD2880_RESULT_ERROR_ARG;
+
+   if (tnrdmd->state != CXD2880_TNRDMD_STATE_ACTIVE)
+   return CXD2880_RESULT_ERROR_SW_STATE;
+
+   if (tnrdmd->sys != CXD2880_DTV_SYS_DVBT)
+   return CXD2880_RESULT_ERROR_SW_STATE;
+
+   if (slvt_freeze_reg(tnrdmd) != CXD2880_RESULT_OK)
+   return CXD2880_RESULT_ERROR_IO;
+
+   if (tnrdmd->io->write_reg(tnrdmd->io, CXD2880_IO_TGT_DMD,
+   0x00, 0x10) != CXD2880_RESULT_OK) {
+   slvt_unfreeze_reg(tnrdmd);
+   return CXD2880_RESULT_ERROR_IO;
+   }
+
+   if (tnrdmd->io->read_regs(tnrdmd->io, CXD2880_IO_TGT_DMD,
+   0x39, rdata, 1) != CXD2880_RESULT_OK) {
+   slvt_unfreeze_reg(tnrdmd);
+   return CXD2880_RESULT_ERROR_IO;
+   }