Re: [Linux-kernel-mentees] [PATCH] media: dvb_dummy_fe.c: add members to dvb_dummy_fe_state

["Daniel W. S. Almeida" <dwlsalmeida@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1592 bytes --]

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.


[-- Attachment #1.2: Type: text/html, Size: 2141 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees