Hi Mauro, thanks for checking out my work! > please don't add fields at the > struct while they're not used, as this makes harder for reviewers to be > sure that we're not adding bloatware at the code. OK. > Please remember that > we want one logical change per patch. > > It means that, if you add a state->frontend_status at the driver, the > patch should implement the entire logic for it. I will keep this in mind. > static int dvb_dummy_fe_sleep(struct dvb_frontend* fe) > { > + > + struct dvb_dummy_fe_state *state = fe->demodulator_priv; > + > + state->sleeping = true; > + > Hmm...what's the sense of adding it? Where are you setting it to false? > Where are you using the sleeping state? I noted some drivers seem to instruct the hardware into a low power state. Take helene, for instance: static int helene_sleep(struct dvb_frontend *fe) { struct helene_priv *priv = fe->tuner_priv; dev_dbg(&priv->i2c->dev, "%s()\n", __func__); helene_enter_power_save(priv); return 0; } static int helene_enter_power_save(struct helene_priv *priv) { dev_dbg(&priv->i2c->dev, "%s()\n", __func__); if (priv->state == STATE_SLEEP) return 0; /* Standby setting for CPU */ helene_write_reg(priv, 0x88, 0x0); /* Standby setting for internal logic block */ helene_write_reg(priv, 0x87, 0xC0); priv->state = STATE_SLEEP; return 0; } As we are emulating some piece of hardware I thought it would be enough to add a simple bool to indicate that the device was sleeping. This patch was a bit convoluted on my part. Let's iron out the issues you pointed out in v2. - Daniel.