Re: [PATCH v2] drivers/media/dvb-frontends: Implement probe/remove for stv6110x

[Tobias Klausmann]

On 29.05.19 20:45, Joe Perches wrote:
On Wed, 2019-05-29 at 18:56 +0200, Tobias Klausmann wrote:
Refactor out the common parts of stv6110x_probe() and stv6110x_attach()
into separate functions.

This provides the needed functionality to use dvb_module_probe() instead
of dvb_attach()!

v2:
- Impovments based on comments by Sean Young
- Fix checkpatch.pl --strict errors
trivia:

diff --git a/drivers/media/dvb-frontends/stv6110x.c 
b/drivers/media/dvb-frontends/stv6110x.c
[]
@@ -333,6 +333,41 @@ static void stv6110x_release(struct dvb_frontend *fe)
        kfree(stv6110x);
  }
  
+void st6110x_init_regs(struct stv6110x_state *stv6110x)
+{
+       u8 default_regs[] = {0x07, 0x11, 0xdc, 0x85, 0x17, 0x01, 0xe6, 0x1e};
static const u8...

+
+       memcpy(stv6110x->regs, default_regs, 8);
        memcpy(stv6110x->regs, default_regs, ARRAY_SIZE(default_regs));

+}
+
+void stv6110x_setup_divider(struct stv6110x_state *stv6110x)
+{
+       switch (stv6110x->config->clk_div) {
+       default:
+       case 1:
+               STV6110x_SETFIELD(stv6110x->regs[STV6110x_CTRL2],
+                                 CTRL2_CO_DIV,
+                                 0);
+               break;
+       case 2:
+               STV6110x_SETFIELD(stv6110x->regs[STV6110x_CTRL2],
+                                 CTRL2_CO_DIV,
+                                 1);
+               break;
+       case 4:
+               STV6110x_SETFIELD(stv6110x->regs[STV6110x_CTRL2],
+                                 CTRL2_CO_DIV,
+                                 2);
+               break;
+       case 8:
+       case 0:
+               STV6110x_SETFIELD(stv6110x->regs[STV6110x_CTRL2],
+                                 CTRL2_CO_DIV,
+                                 3);
+               break;
+       }
+}
Probably more sensible (and smaller object code) written using
an automatic like:

{
        int div;

        switch (stv6110x->config->clk_div) {
        case 8:
                div = 3;
                break;
        case 4:
                div = 2;
                break;
        case 2:
                div = 1;
                break;
        case 1:
        default:
                div = 0;
                break;
        }
        STV6110x_SETFIELD(stv6110x->regs[STV6110x_CTRL2], CTRL2_CO_DIV, div);
}

diff --git a/drivers/media/dvb-frontends/stv6110x_priv.h 
b/drivers/media/dvb-frontends/stv6110x_priv.h
[]
@@ -54,11 +54,12 @@
  #define REFCLOCK_MHz                          (stv6110x->config->refclk / 
1000000)
  
  struct stv6110x_state {
+       struct dvb_frontend             *frontend;
        struct i2c_adapter              *i2c;
        const struct stv6110x_config    *config;
        u8                              regs[8];
Perhaps this 8 should be a define?



Hi,

thanks for the comments! If really desired i can change the code 
further, adapting to your comments, but note that the code was 
essentially just moved around to cater to both _probe() and attach(), 
intentionally leaving it as it was before the patch!

Greetings,

Tobias