Re: [PATCH 1/4] [media] dvb-frontends: MaxLinear MxL5xx DVB-S/S2 tuner-demodulator driver

[Daniel Scheller]

Am Mon, 10 Jul 2017 10:41:03 +0200
schrieb Ralph Metzler :

> Daniel Scheller writes:
>  > From: Daniel Scheller 
>  > 
>  > This adds the frontend driver for the MaxLinear MxL5xx family of
>  > tuner- demodulators, as used on Digital Devices MaxS4/8
>  > four/eight-tuner cards.
>  > 
>  > The driver was picked from the dddvb vendor driver package and -
>  > judging solely from the diff - has undergone a 100% rework:
>  > 
>  >  - Silly #define's used to pass multiple values to functions were
>  >expanded. This resulted in macro/register names not being usable
>  >anymore for such occurences, but makes the code WAY more read-,
>  >understand- and maintainable.  
> 
> OK, but why did you also replace all kinds of register value defines
> with numerical values? This makes the driver much less comprehensible.

I actually tried to put named identifiers back in as I don't feel very
comfortable with just plain hex numbers, there aren't even that many
registers that get accessed. But as I tried to do (based on the
original _regs.h), I quickly found out that one register can belong to
many identifiers, with the only difference being the bitshift and
bitcount, so without knowing the correct names this ends up in lottery
style, which isn't good either. If you have some documentation on this
and can provide that, we can put names back in place of course.

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst

[PATCH 1/4] [media] dvb-frontends: MaxLinear MxL5xx DVB-S/S2 tuner-demodulator driver

[Ralph Metzler]

Daniel Scheller writes:
 > From: Daniel Scheller 
 > 
 > This adds the frontend driver for the MaxLinear MxL5xx family of tuner-
 > demodulators, as used on Digital Devices MaxS4/8 four/eight-tuner cards.
 > 
 > The driver was picked from the dddvb vendor driver package and - judging
 > solely from the diff - has undergone a 100% rework:
 > 
 >  - Silly #define's used to pass multiple values to functions were
 >expanded. This resulted in macro/register names not being usable
 >anymore for such occurences, but makes the code WAY more read-,
 >understand- and maintainable.

OK, but why did you also replace all kinds of register value defines
with numerical values? This makes the driver much less comprehensible.

[PATCH 1/4] [media] dvb-frontends: MaxLinear MxL5xx DVB-S/S2 tuner-demodulator driver

[Daniel Scheller]

From: Daniel Scheller 

This adds the frontend driver for the MaxLinear MxL5xx family of tuner-
demodulators, as used on Digital Devices MaxS4/8 four/eight-tuner cards.

The driver was picked from the dddvb vendor driver package and - judging
solely from the diff - has undergone a 100% rework:

 - Silly #define's used to pass multiple values to functions were
   expanded. This resulted in macro/register names not being usable
   anymore for such occurences, but makes the code WAY more read-,
   understand- and maintainable.
 - CamelCase was changed to kernel_case
 - All typedef were removed
 - Overall code style was fixed, besides >80char lines in _defs.h and
   _regs.h, checkpatch is happy.
 - Also, signal stat acquisition was made to comply with the DVB API
   ways to do these things.

Permission to reuse and mainline the driver code was formally granted by
Ralph Metzler .

Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/Kconfig   |9 +
 drivers/media/dvb-frontends/Makefile  |1 +
 drivers/media/dvb-frontends/mxl5xx.c  | 1873 +
 drivers/media/dvb-frontends/mxl5xx.h  |   41 +
 drivers/media/dvb-frontends/mxl5xx_defs.h |  731 +++
 drivers/media/dvb-frontends/mxl5xx_regs.h |  367 ++
 6 files changed, 3022 insertions(+)
 create mode 100644 drivers/media/dvb-frontends/mxl5xx.c
 create mode 100644 drivers/media/dvb-frontends/mxl5xx.h
 create mode 100644 drivers/media/dvb-frontends/mxl5xx_defs.h
 create mode 100644 drivers/media/dvb-frontends/mxl5xx_regs.h

diff --git a/drivers/media/dvb-frontends/Kconfig 
b/drivers/media/dvb-frontends/Kconfig
index d2d3160abdf7..2631d0e0a024 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -53,6 +53,15 @@ config DVB_STV6111
 
  Say Y when you want to support these frontends.
 
+config DVB_MXL5XX
+   tristate "MaxLinear MxL5xx based tuner-demodulators"
+   depends on DVB_CORE && I2C
+   default m if !MEDIA_SUBDRV_AUTOSELECT
+   help
+ MaxLinear MxL5xx family of DVB-S/S2 tuners/demodulators.
+
+ Say Y when you want to support these frontends.
+
 config DVB_M88DS3103
tristate "Montage Technology M88DS3103"
depends on DVB_CORE && I2C && I2C_MUX
diff --git a/drivers/media/dvb-frontends/Makefile 
b/drivers/media/dvb-frontends/Makefile
index e8bf1d873485..f45f6a4a4371 100644
--- a/drivers/media/dvb-frontends/Makefile
+++ b/drivers/media/dvb-frontends/Makefile
@@ -112,6 +112,7 @@ obj-$(CONFIG_DVB_DRXK) += drxk.o
 obj-$(CONFIG_DVB_TDA18271C2DD) += tda18271c2dd.o
 obj-$(CONFIG_DVB_STV0910) += stv0910.o
 obj-$(CONFIG_DVB_STV6111) += stv6111.o
+obj-$(CONFIG_DVB_MXL5XX) += mxl5xx.o
 obj-$(CONFIG_DVB_SI2165) += si2165.o
 obj-$(CONFIG_DVB_A8293) += a8293.o
 obj-$(CONFIG_DVB_SP2) += sp2.o
diff --git a/drivers/media/dvb-frontends/mxl5xx.c 
b/drivers/media/dvb-frontends/mxl5xx.c
new file mode 100644
index ..676c96c216c3
--- /dev/null
+++ b/drivers/media/dvb-frontends/mxl5xx.c
@@ -0,0 +1,1873 @@
+/*
+ * Driver for the MaxLinear MxL5xx family of tuners/demods
+ *
+ * Copyright (C) 2014-2015 Ralph Metzler 
+ * Marcus Metzler 
+ * developed for Digital Devices GmbH
+ *
+ * based on code:
+ * Copyright (c) 2011-2013 MaxLinear, Inc. All rights reserved
+ * which was released under GPL V2
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "dvb_frontend.h"
+#include "mxl5xx.h"
+#include "mxl5xx_regs.h"
+#include "mxl5xx_defs.h"
+
+#define BYTE0(v) ((v >>  0) & 0xff)
+#define BYTE1(v) ((v >>  8) & 0xff)
+#define BYTE2(v) ((v >> 16) & 0xff)
+#define BYTE3(v) ((v >> 24) & 0xff)
+
+LIST_HEAD(mxllist);
+
+struct mxl_base {
+   struct list_head mxllist;
+   struct list_head mxls;
+
+   u8   adr;
+   struct i2c_adapter  *i2c;
+
+   u32  count;
+   u32  type;
+   u32  sku_type;
+   u32  chipversion;
+   u32  clock;
+   u32  fwversion;
+
+   u8  *ts_map;
+   u8   can_clkout;
+   u8   chan_bond;
+   u8   demod_num;
+   u8   tuner_num;
+
+   unsigned longnext_tune;
+