RE: [PATCH v4 07/12] [media] cxd2880: Add top level of the driver

[Takiguchi, Yasunari]

Hi, Mauro

> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
> 
> Same comments as on other patches: use SPDX and dev_foo() for printing
> messages.

About printing messages pr_fmt, I also replied a comment to [PATCH v4 02/12] 
[media] cxd2880-spi: Add support for CXD2880 SPI interface.
Please refer to the comment.

Thanks,
Takiguchi

Re: [PATCH v4 07/12] [media] cxd2880: Add top level of the driver

[Mauro Carvalho Chehab]

Em Fri, 13 Oct 2017 15:09:34 +0900
 escreveu:

> 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 
> ---
> 
> [Change list]
> Changes in V4
>drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>   -modified typo "inavlid" to "invalid" at pr_err
>   -removed unnecessary initialization at variable declaration
>   -removed unnecessary brace {}
>   -changed to use cxd2880_dvbt_tune and cxd2880_dvbt2_tune 
>instead of cxd2880_integ_dvbt_tune and cxd2880_integ_dvbt2_tune
>(because we changed it so that demodulator does not 
>  wait for locking the signal.) 
> 
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>   -adjusted indent spaces
>   -modified debugging code
>   -removed unnecessary cast
>   -modified return code
>   -modified coding style of if() 
>   -modified about measurement period of PER/BER.
>   -changed hexadecimal code to lower case. 
> 
>  drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 2019 
> +
>  1 file changed, 2019 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 ..c82aaf0c1597
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> @@ -0,0 +1,2019 @@
> +/*
> + * 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 .
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__

Same comments as on other patches: use SPDX and dev_foo() for printing 
messages.

> +
> +#include 
> +
> +#include "dvb_frontend.h"
> +#include "dvb_math.h"
> +
> +#include "cxd2880.h"
> +#include "cxd2880_tnrdmd_mon.h"
> +#include "cxd2880_tnrdmd_dvbt2_mon.h"
> +#include "cxd2880_tnrdmd_dvbt_mon.h"
> +#include "cxd2880_integ.h"
> +#include "cxd2880_tnrdmd_dvbt2.h"
> +#include "cxd2880_tnrdmd_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 */
> + unsigned long pre_ber_update;
> + unsigned long pre_ber_interval;
> + unsigned long post_ber_update;
> + unsigned long post_ber_interval;
> + unsigned long ucblock_update;
> + unsigned long ucblock_interval;
> + enum fe_status s;
> +};
> +
> +static int cxd2880_pre_bit_err_t(
> + struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
> + u32 *pre_bit_count)
> +{
> + u8 rdata[2];
> + int ret;
> +
> + if ((!tnrdmd) || (!pre_bit_err) || (!pre_bit_count))
> + return -EINVAL;
> +
> + if (tnrdmd->diver_mode == CXD2880_TNRDMD_DIVERMODE_SUB)
> + return -EINVAL;
> +
> + if (tnrdmd->state != CXD2880_TNRDMD_STATE_ACTIVE)
> + return -EPERM;
> +
> + if 

RE: [PATCH v4 07/12] [media] cxd2880: Add top level of the driver

[Takiguchi, Yasunari]

Dear Sean

Hi, Thanks for your review.

We will refer to your comments and consider how to respond for them.

> > +   u8 rdata[2];
> > +   int ret;
> > +
> > +   if ((!tnrdmd) || (!pre_bit_err) || (!pre_bit_count))
> > +   return -EINVAL;
> > +
> > +   if (tnrdmd->diver_mode == CXD2880_TNRDMD_DIVERMODE_SUB)
> > +   return -EINVAL;
> 
> divermode: this should say drivermode, correct?

diver_mode is not typo, because cxd2880 has diversity function.

> > +MODULE_DESCRIPTION(
> > +"Sony CXD2880 DVB-T2/T tuner + demodulator drvier");
> 
> drvier => driver
Yes. It is typo. We will also re-check other patch files.

Thanks
Takiguchi

Re: [PATCH v4 07/12] [media] cxd2880: Add top level of the driver

[Sean Young]

Hello,

Many thanks for the driver.

The ./scripts/checkpatch.pl with --strict has many warnings, some of which might
be nice to have cleaned up.

There are some very minor comments below. Similar constructs are in other files 
too.

After reading and understanding through all the code, I think the driver looks
very good, and is ready for merging except for the minor things listed.

Thank you for your submission.

Regards,

Sean


On Fri, Oct 13, 2017 at 03:09:34PM +0900, yasunari.takigu...@sony.com wrote:
> 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 
> ---
> 
> [Change list]
> Changes in V4
>drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>   -modified typo "inavlid" to "invalid" at pr_err
>   -removed unnecessary initialization at variable declaration
>   -removed unnecessary brace {}
>   -changed to use cxd2880_dvbt_tune and cxd2880_dvbt2_tune 
>instead of cxd2880_integ_dvbt_tune and cxd2880_integ_dvbt2_tune
>(because we changed it so that demodulator does not 
>  wait for locking the signal.) 
> 
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
>   -adjusted indent spaces
>   -modified debugging code
>   -removed unnecessary cast
>   -modified return code
>   -modified coding style of if() 
>   -modified about measurement period of PER/BER.
>   -changed hexadecimal code to lower case. 
> 
>  drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 2019 
> +
>  1 file changed, 2019 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 ..c82aaf0c1597
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
> @@ -0,0 +1,2019 @@
> +/*
> + * 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 .
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
> +
> +#include 
> +
> +#include "dvb_frontend.h"
> +#include "dvb_math.h"
> +
> +#include "cxd2880.h"
> +#include "cxd2880_tnrdmd_mon.h"
> +#include "cxd2880_tnrdmd_dvbt2_mon.h"
> +#include "cxd2880_tnrdmd_dvbt_mon.h"
> +#include "cxd2880_integ.h"
> +#include "cxd2880_tnrdmd_dvbt2.h"
> +#include "cxd2880_tnrdmd_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 */
> + unsigned long pre_ber_update;
> + unsigned long pre_ber_interval;
> + unsigned long post_ber_update;
> + unsigned long post_ber_interval;
> + unsigned long ucblock_update;
> + unsigned long ucblock_interval;
> + enum fe_status s;
> +};
> +
> +static int cxd2880_pre_bit_err_t(
> + struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
> + u32 *pre_bit_count)

[PATCH v4 07/12] [media] cxd2880: Add top level of the driver

[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 
---

[Change list]
Changes in V4
   drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
  -modified typo "inavlid" to "invalid" at pr_err
  -removed unnecessary initialization at variable declaration
  -removed unnecessary brace {}
  -changed to use cxd2880_dvbt_tune and cxd2880_dvbt2_tune 
   instead of cxd2880_integ_dvbt_tune and cxd2880_integ_dvbt2_tune
   (because we changed it so that demodulator does not 
 wait for locking the signal.) 

Changes in V3
   drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
  -adjusted indent spaces
  -modified debugging code
  -removed unnecessary cast
  -modified return code
  -modified coding style of if() 
  -modified about measurement period of PER/BER.
  -changed hexadecimal code to lower case. 

 drivers/media/dvb-frontends/cxd2880/cxd2880_top.c | 2019 +
 1 file changed, 2019 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 ..c82aaf0c1597
--- /dev/null
+++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_top.c
@@ -0,0 +1,2019 @@
+/*
+ * 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 .
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
+
+#include 
+
+#include "dvb_frontend.h"
+#include "dvb_math.h"
+
+#include "cxd2880.h"
+#include "cxd2880_tnrdmd_mon.h"
+#include "cxd2880_tnrdmd_dvbt2_mon.h"
+#include "cxd2880_tnrdmd_dvbt_mon.h"
+#include "cxd2880_integ.h"
+#include "cxd2880_tnrdmd_dvbt2.h"
+#include "cxd2880_tnrdmd_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 */
+   unsigned long pre_ber_update;
+   unsigned long pre_ber_interval;
+   unsigned long post_ber_update;
+   unsigned long post_ber_interval;
+   unsigned long ucblock_update;
+   unsigned long ucblock_interval;
+   enum fe_status s;
+};
+
+static int cxd2880_pre_bit_err_t(
+   struct cxd2880_tnrdmd *tnrdmd, u32 *pre_bit_err,
+   u32 *pre_bit_count)
+{
+   u8 rdata[2];
+   int ret;
+
+   if ((!tnrdmd) || (!pre_bit_err) || (!pre_bit_count))
+   return -EINVAL;
+
+   if (tnrdmd->diver_mode == CXD2880_TNRDMD_DIVERMODE_SUB)
+   return -EINVAL;
+
+   if (tnrdmd->state != CXD2880_TNRDMD_STATE_ACTIVE)
+   return -EPERM;
+
+   if (tnrdmd->sys != CXD2880_DTV_SYS_DVBT)
+   return -EPERM;
+
+   ret = slvt_freeze_reg(tnrdmd);
+   if (ret)
+   return ret;
+
+   ret = tnrdmd->io->write_reg(tnrdmd->io,
+   CXD2880_IO_TGT_DMD,
+   0x00, 0x10);
+   if (ret) {
+