Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-09-07 Thread Mauro Carvalho Chehab 
Em Thu, 7 Sep 2017 19:12:57 +0900
"Takiguchi, Yasunari"  escreveu:

> Dear Mauro
> 
> Thanks for your review and reply.
> 
> We are going to discuss how to change our code with your comments internally.
> 
> I reply for your  2 comments,
> 
> >> [Change list]
> >> Changes in V3
> >>drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> >>   -removed code relevant to ISDB-T
> > 
> > Just curiosity here: why is it removed?
> We decided to withhold the ISDB-T functionality as it contains some company 
> proprietary code.

I'm sorry to hear. I hope that such code could be released
on some future.

> >> +  if (ret)
> >> +  return ret;
> >> +  if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl) {
> >> +  data[0] = 0x01;
> >> +  data[1] = 0x01;
> >> +  data[2] = 0x01;
> >> +  data[3] = 0x01;
> >> +  data[4] = 0x01;
> >> +  data[5] = 0x01;
> >> +  } else {
> >> +  data[0] = 0x00;
> >> +  data[1] = 0x00;
> >> +  data[2] = 0x00;
> >> +  data[3] = 0x00;
> >> +  data[4] = 0x00;
> >> +  data[5] = 0x00;
> >> +  }
> > 
> > Instead, just do:
> > 
> > if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
> > memset(data, 0x01, sizeof(data));
> > else
> > memset(data, 0x00, sizeof(data));
> > 
> >> +  ret = tnr_dmd->io->write_regs(tnr_dmd->io,
> >> +CXD2880_IO_TGT_SYS,
> >> +0xef, data, 6);
> >> +  if (ret)
> >> +  return ret;
> >> +
> >> +  ret = tnr_dmd->io->write_reg(tnr_dmd->io,
> >> +   CXD2880_IO_TGT_DMD,
> >> +   0x00, 0x2d);
> >> +  if (ret)
> >> +  return ret;
> > 
> >> +  if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
> >> +  data[0] = 0x00;
> >> +  else
> >> +  data[0] = 0x01;
> > 
> > Not actually needed, as the previous logic already set data[0]
> > accordingly.
> > 
> >> +  ret = tnr_dmd->io->write_reg(tnr_dmd->io,
> >> +   CXD2880_IO_TGT_DMD,
> >> +   0xb1, data[0]);
> 
> In this case、logic of data[0]( logic of if() ) is different from that of 
> previous one.
> And with setting register for address 0xb1, a bug might occur in the future, 
> if our software specification (sequence) is changed.
> So we would like to keep setting value of data[0] for address 0xb1.

OK. Better to document it then, as otherwise someone might
end by sending cleanup patches that would touch it.

> 
> Thanks,
> Takiguchi



Thanks,
Mauro

Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-09-07 Thread Takiguchi, Yasunari 
Dear Mauro

Thanks for your review and reply.

We are going to discuss how to change our code with your comments internally.

I reply for your  2 comments,

>> [Change list]
>> Changes in V3
>>drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>>   -removed code relevant to ISDB-T
> 
> Just curiosity here: why is it removed?
We decided to withhold the ISDB-T functionality as it contains some company 
proprietary code.


>> +if (ret)
>> +return ret;
>> +if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl) {
>> +data[0] = 0x01;
>> +data[1] = 0x01;
>> +data[2] = 0x01;
>> +data[3] = 0x01;
>> +data[4] = 0x01;
>> +data[5] = 0x01;
>> +} else {
>> +data[0] = 0x00;
>> +data[1] = 0x00;
>> +data[2] = 0x00;
>> +data[3] = 0x00;
>> +data[4] = 0x00;
>> +data[5] = 0x00;
>> +}
> 
> Instead, just do:
> 
>   if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
>   memset(data, 0x01, sizeof(data));
>   else
>   memset(data, 0x00, sizeof(data));
> 
>> +ret = tnr_dmd->io->write_regs(tnr_dmd->io,
>> +  CXD2880_IO_TGT_SYS,
>> +  0xef, data, 6);
>> +if (ret)
>> +return ret;
>> +
>> +ret = tnr_dmd->io->write_reg(tnr_dmd->io,
>> + CXD2880_IO_TGT_DMD,
>> + 0x00, 0x2d);
>> +if (ret)
>> +return ret;
> 
>> +if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
>> +data[0] = 0x00;
>> +else
>> +data[0] = 0x01;
> 
> Not actually needed, as the previous logic already set data[0]
> accordingly.
> 
>> +ret = tnr_dmd->io->write_reg(tnr_dmd->io,
>> + CXD2880_IO_TGT_DMD,
>> + 0xb1, data[0]);

In this case、logic of data[0]( logic of if() ) is different from that of 
previous one.
And with setting register for address 0xb1, a bug might occur in the future, 
if our software specification (sequence) is changed.
So we would like to keep setting value of data[0] for address 0xb1.

Thanks,
Takiguchi

Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-08-27 Thread Mauro Carvalho Chehab 
Em Sun, 27 Aug 2017 11:45:44 -0300
Mauro Carvalho Chehab  escreveu:

> Em Wed, 16 Aug 2017 13:37:14 +0900
>  escreveu:
> 
> > From: Yasunari Takiguchi 
> > 
> > This part of the driver has the main routines to handle
> > the tuner and demodulator functionality.  The tnrdmd_mon.* files
> > have monitor functions for the driver.
> > This is part of the Sony CXD2880 DVB-T2/T tuner + demodulator driver.
> 
> This series is on a much better state. Thanks!
> 
> Patches 1-4 sound ok to me.
> 
> Still, there are a few issues on this patch. See below.
> 
> > 
> > [Change list]
> > Changes in V3
> >drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> >   -removed code relevant to ISDB-T

Forgot to mention... Just a small detail: the change list is important for
us to understand the differences between submitted versions, but it makes
no sense to track them when patches get merged upstream, as the previous
versions of the patches won't be there at the git history.

So, please put change lists after a blank line with just "---", e. g.:


From: Yasunari Takiguchi 

This part of the driver has the main routines to handle
the tuner and demodulator functionality.  The tnrdmd_mon.* files
have monitor functions for the driver.
This is part of the Sony CXD2880 DVB-T2/T tuner + demodulator driver.

---

(change list and whatever other notes that are relevant to the
 patch reviewers but won't be merged upstream)

The reason is simple: Kernel maintainers' scripts will just remove
everything after the "---" line when applying the series ;-)

Thanks,
Mauro

Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-08-27 Thread Mauro Carvalho Chehab 
Em Wed, 16 Aug 2017 13:37:14 +0900
 escreveu:

> From: Yasunari Takiguchi 
> 
> This part of the driver has the main routines to handle
> the tuner and demodulator functionality.  The tnrdmd_mon.* files
> have monitor functions for the driver.
> This is part of the Sony CXD2880 DVB-T2/T tuner + demodulator driver.

This series is on a much better state. Thanks!

Patches 1-4 sound ok to me.

Still, there are a few issues on this patch. See below.

> 
> [Change list]
> Changes in V3
>drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>   -removed code relevant to ISDB-T

Just curiosity here: why is it removed?

>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
>   -removed unnecessary cast
>   -removed code relevant to ISDB-T
>   -changed CXD2880_SLEEP to usleep_range
>   -changed cxd2880_memset to memset 
>   -changed cxd2880_atomic_set to atomic_set
>   -modified return code
>   -modified coding style of if()
>   -changed to use const values at writing a lot of registers 
>with a command. 
>   -changed hexadecimal code to lower case. 
>   -adjusted of indent spaces
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
>   -removed code relevant to ISDB-T
>   -changed cxd2880_atomic struct to atomic_t
>   -modified return code
>   -changed hexadecimal code to lower case. 
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>   -updated version information
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
>   -changed CXD2880_SLEEP to usleep_range
>   -removed unnecessary cast
>   -modified return code
>   -modified coding style of if() 
>   -changed hexadecimal code to lower case. 
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h
>   -modified return code
> 
> Changes in V2
>drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>   -updated version information
> 
> 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_dtv.h  |   46 +
>  .../media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c   | 4030 
> 
>  .../media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h   |  391 ++
>  .../cxd2880/cxd2880_tnrdmd_driver_version.h|   29 +
>  .../dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c |  221 ++
>  .../dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h |   52 +
>  6 files changed, 4769 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
>  create mode 100644 
> drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h 
> b/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> new file mode 100644
> index ..2d35d3990060
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> @@ -0,0 +1,46 @@
> +/*
> + * cxd2880_dtv.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * DTV related definitions
> + *
> + * 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 .
> + */
> +
> +#ifndef CXD2880_DTV_H
> +#define CXD2880_DTV_H
> +
> +enum

[PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-08-15 Thread Yasunari.Takiguchi 
From: Yasunari Takiguchi 

This part of the driver has the main routines to handle
the tuner and demodulator functionality.  The tnrdmd_mon.* files
have monitor functions for the driver.
This is part of the Sony CXD2880 DVB-T2/T tuner + demodulator driver.

[Change list]
Changes in V3
   drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
  -removed code relevant to ISDB-T
   drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
  -removed unnecessary cast
  -removed code relevant to ISDB-T
  -changed CXD2880_SLEEP to usleep_range
  -changed cxd2880_memset to memset 
  -changed cxd2880_atomic_set to atomic_set
  -modified return code
  -modified coding style of if()
  -changed to use const values at writing a lot of registers 
   with a command. 
  -changed hexadecimal code to lower case. 
  -adjusted of indent spaces
   drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
  -removed code relevant to ISDB-T
  -changed cxd2880_atomic struct to atomic_t
  -modified return code
  -changed hexadecimal code to lower case. 
   drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
  -updated version information
   drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
  -changed CXD2880_SLEEP to usleep_range
  -removed unnecessary cast
  -modified return code
  -modified coding style of if() 
  -changed hexadecimal code to lower case. 
   drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h
  -modified return code

Changes in V2
   drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
  -updated version information

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_dtv.h  |   46 +
 .../media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c   | 4030 
 .../media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h   |  391 ++
 .../cxd2880/cxd2880_tnrdmd_driver_version.h|   29 +
 .../dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c |  221 ++
 .../dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h |   52 +
 6 files changed, 4769 insertions(+)
 create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
 create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c
 create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.h
 create mode 100644 
drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_driver_version.h
 create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.c
 create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd_mon.h

diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h 
b/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
new file mode 100644
index ..2d35d3990060
--- /dev/null
+++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
@@ -0,0 +1,46 @@
+/*
+ * cxd2880_dtv.h
+ * Sony CXD2880 DVB-T2/T tuner + demodulator driver
+ * DTV related definitions
+ *
+ * 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 .
+ */
+
+#ifndef CXD2880_DTV_H
+#define CXD2880_DTV_H
+
+enum cxd2880_dtv_sys {
+   CXD2880_DTV_SYS_UNKNOWN,
+   CXD2880_DTV_SYS_DVBT,
+   CXD2880_DTV_SYS_DVBT2,
+   CXD2880_DTV_SYS_ANY
+};
+
+enum cxd2880_dtv_bandwidth {
+   CXD2880_DTV_BW_UNKNOWN = 0,
+   CXD2880_DTV_BW_1_7_MHZ = 1,
+   CXD2880_DTV_BW_5_MHZ = 5,
+   CXD2880_DTV_BW_6_MHZ = 6,
+   CXD2880_DTV_BW_7_MHZ = 7,
+   CXD2880_DTV_BW_8_MHZ = 8
+};
+
+#endif
diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_tnrdmd.c