All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean-Francois Moine <moinejf@free.fr>
To: Jyri Sarha <jsarha@ti.com>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Lars-Peter Clausen <lars@metafoo.de>,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	alsa-devel@alsa-project.org, dri-devel@lists.freedesktop.org,
	broonie@kernel.org, peter.ujfalusi@ti.com, detheridge@ti.com
Subject: Re: [alsa-devel] [PATCH RFC v3 0/8] Beaglebone-Black HDMI audio
Date: Thu, 30 Jan 2014 19:00:11 +0100	[thread overview]
Message-ID: <20140130190011.3849cf37@armhf> (raw)
In-Reply-To: <52EA43A8.9050909@ti.com>

On Thu, 30 Jan 2014 14:20:56 +0200
Jyri Sarha <jsarha@ti.com> wrote:

> I am having trouble getting the tda998x-codec working on BBB. The 
> problem is I do not have a DT node for the tda998x driver. Instead I 
> have tilcdc-slave node that provides pdata for the tda-driver.
> 
> I am thinking of solving the problem by adding a reference to the 
> i2c-adapter hosting tda998x as an optional DT property to the codec 
> node. I could then dig the driver instance from the i2c adapter's 
> children. Any better ideas?

I better think about a 'normal' DT definition:

- in the DT, define the tda998x in a i2c subnode:

  &i2c0 {
	tda998x: hdmi-encoder {
		compatible = "nxp,tda998x";
		reg = <0x70>;
		/* the video ports are OK by default */
		/* define the interrupt if you want to use it */
	};
  };

- in tilcdc_slave.c, in the function slave_encoder_create(), instead of
  using drm_i2c_encoder_init(), do quite the same, but without calling
  request_module() nor i2c_new_device().

  This can be done as follows (the code is not compiled):

--------------------8<---------------------
static struct drm_encoder *slave_encoder_create(struct drm_device *dev,
		struct slave_module *mod)
{
	struct slave_encoder *slave_encoder;
	struct drm_encoder *encoder;
	int ret;
/* ------ added ------ */
	struct device_node *np;
	static const struct of_device_id tda_dt[] = {
		{ .compatible = "nxp,tda998x" },
		{ },
	};
/* ------ end added ------ */

	... no change ...

	 drm_encoder_helper_add(encoder, &slave_encoder_helper_funcs);

/* ------ added ------ */

	/* search the tda998x device */
	np = of_find_matching_node_and_match(NULL, tda_dt, NULL);
	if (np && of_device_is_available(np)) {
		struct i2c_client *i2c_client;
		struct drm_i2c_encoder_driver *encoder_drv;
		struct module *module;

					/* the tda998x is in the DT */

		i2c_client = of_find_i2c_device_by_node(np);
		of_node_put(np);
		if (!i2c_client) {
			dev_err(dev->dev, "no tda998x i2c client\n");
			goto fail;
		}

		to_encoder_slave(encoder)->bus_priv = i2c_client;

		encoder_drv = to_drm_i2c_encoder_driver(
				to_i2c_driver(i2c_client->dev.driver));

		/* lock the tda998x module in memory */
		module = to_i2c_driver(i2c_client->dev.driver)->driver.owner;
		if (!module || !try_module_get(module)) {
			dev_err(dev->dev, "cannot get module %s\n", module->name);
			goto fail;
		}

		ret = encoder_drv->encoder_init(i2c_client, dev, encoder_slave);
		if (ret < 0) {
			dev_err(dev->dev, "slave encoder init failed\n");
			module_put(module);
			goto fail;
		}
					/* set_config is useless */
		return encoder;
	}

/* ------ end added ------ */

	ret = drm_i2c_encoder_init(dev, to_encoder_slave(encoder), mod->i2c, &info);
	if (ret)
		goto fail;

	return encoder;

fail:
	slave_encoder_destroy(encoder);
	return NULL;
}
--------------------8<---------------------

When the tda998x is in the DT, the i2c_client is already created.
It must not be freed, and so, the function drm_i2c_encoder_destroy()
must not be called. But, the module must be explicitly unlocked in
slave_encoder_destroy(), and then, there must be some flag in the
structures for this job to be done...

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: moinejf@free.fr (Jean-Francois Moine)
To: linux-arm-kernel@lists.infradead.org
Subject: [alsa-devel] [PATCH RFC v3 0/8] Beaglebone-Black HDMI audio
Date: Thu, 30 Jan 2014 19:00:11 +0100	[thread overview]
Message-ID: <20140130190011.3849cf37@armhf> (raw)
In-Reply-To: <52EA43A8.9050909@ti.com>

On Thu, 30 Jan 2014 14:20:56 +0200
Jyri Sarha <jsarha@ti.com> wrote:

> I am having trouble getting the tda998x-codec working on BBB. The 
> problem is I do not have a DT node for the tda998x driver. Instead I 
> have tilcdc-slave node that provides pdata for the tda-driver.
> 
> I am thinking of solving the problem by adding a reference to the 
> i2c-adapter hosting tda998x as an optional DT property to the codec 
> node. I could then dig the driver instance from the i2c adapter's 
> children. Any better ideas?

I better think about a 'normal' DT definition:

- in the DT, define the tda998x in a i2c subnode:

  &i2c0 {
	tda998x: hdmi-encoder {
		compatible = "nxp,tda998x";
		reg = <0x70>;
		/* the video ports are OK by default */
		/* define the interrupt if you want to use it */
	};
  };

- in tilcdc_slave.c, in the function slave_encoder_create(), instead of
  using drm_i2c_encoder_init(), do quite the same, but without calling
  request_module() nor i2c_new_device().

  This can be done as follows (the code is not compiled):

--------------------8<---------------------
static struct drm_encoder *slave_encoder_create(struct drm_device *dev,
		struct slave_module *mod)
{
	struct slave_encoder *slave_encoder;
	struct drm_encoder *encoder;
	int ret;
/* ------ added ------ */
	struct device_node *np;
	static const struct of_device_id tda_dt[] = {
		{ .compatible = "nxp,tda998x" },
		{ },
	};
/* ------ end added ------ */

	... no change ...

	 drm_encoder_helper_add(encoder, &slave_encoder_helper_funcs);

/* ------ added ------ */

	/* search the tda998x device */
	np = of_find_matching_node_and_match(NULL, tda_dt, NULL);
	if (np && of_device_is_available(np)) {
		struct i2c_client *i2c_client;
		struct drm_i2c_encoder_driver *encoder_drv;
		struct module *module;

					/* the tda998x is in the DT */

		i2c_client = of_find_i2c_device_by_node(np);
		of_node_put(np);
		if (!i2c_client) {
			dev_err(dev->dev, "no tda998x i2c client\n");
			goto fail;
		}

		to_encoder_slave(encoder)->bus_priv = i2c_client;

		encoder_drv = to_drm_i2c_encoder_driver(
				to_i2c_driver(i2c_client->dev.driver));

		/* lock the tda998x module in memory */
		module = to_i2c_driver(i2c_client->dev.driver)->driver.owner;
		if (!module || !try_module_get(module)) {
			dev_err(dev->dev, "cannot get module %s\n", module->name);
			goto fail;
		}

		ret = encoder_drv->encoder_init(i2c_client, dev, encoder_slave);
		if (ret < 0) {
			dev_err(dev->dev, "slave encoder init failed\n");
			module_put(module);
			goto fail;
		}
					/* set_config is useless */
		return encoder;
	}

/* ------ end added ------ */

	ret = drm_i2c_encoder_init(dev, to_encoder_slave(encoder), mod->i2c, &info);
	if (ret)
		goto fail;

	return encoder;

fail:
	slave_encoder_destroy(encoder);
	return NULL;
}
--------------------8<---------------------

When the tda998x is in the DT, the i2c_client is already created.
It must not be freed, and so, the function drm_i2c_encoder_destroy()
must not be called. But, the module must be explicitly unlocked in
slave_encoder_destroy(), and then, there must be some flag in the
structures for this job to be done...

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

  reply	other threads:[~2014-01-30 18:00 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-27 15:37 [PATCH RFC v3 0/8] Beaglebone-Black HDMI audio Jyri Sarha
2014-01-27 15:37 ` Jyri Sarha
2014-01-27 15:37 ` [PATCH RFC v3 1/8] clk: add gpio controlled clock Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 15:37 ` [PATCH RFC v3 2/8] ASoC: davinci-evm: Add named clock reference to DT bindings Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 18:10   ` Mark Brown
2014-01-27 18:10     ` Mark Brown
2014-01-27 15:37 ` [PATCH RFC v3 3/8] ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 20:46   ` Mark Brown
2014-01-27 20:46     ` Mark Brown
2014-01-27 15:37 ` [PATCH RFC v3 4/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 20:49   ` Mark Brown
2014-01-27 20:49     ` Mark Brown
2014-01-28  7:47     ` Jyri Sarha
2014-01-28  7:47       ` Jyri Sarha
2014-01-27 15:37 ` [PATCH RFC v3 5/8] ASoC: davinci: HDMI audio build for AM33XX and TDA998x Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 15:37 ` [PATCH RFC v3 6/8] drm/tilcdc: Add I2C HDMI audio config for tda998x Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 15:37 ` [PATCH RFC v3 7/8] ARM: OMAP2+: omap2plus_defconfig: Enable tilcdc and TDA998X HDMI support Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 15:37 ` [PATCH RFC v3 8/8] ARM: OMAP2+: omap2plus_defconfig: Enable BeagleBone Black HDMI audio support Jyri Sarha
2014-01-27 15:37   ` Jyri Sarha
2014-01-27 15:51 ` [PATCH RFC v3 0/8] Beaglebone-Black HDMI audio Lars-Peter Clausen
2014-01-27 15:51   ` [alsa-devel] " Lars-Peter Clausen
2014-01-27 16:17   ` Jyri Sarha
2014-01-27 16:17     ` [alsa-devel] " Jyri Sarha
2014-01-27 16:32     ` Lars-Peter Clausen
2014-01-27 16:32       ` Lars-Peter Clausen
2014-01-27 19:40       ` Jyri Sarha
2014-01-27 19:40         ` [alsa-devel] " Jyri Sarha
2014-01-27 19:47         ` Lars-Peter Clausen
2014-01-27 19:47           ` Lars-Peter Clausen
2014-01-27 18:06     ` Jean-Francois Moine
2014-01-27 18:06       ` Jean-Francois Moine
2014-01-27 19:31       ` Jyri Sarha
2014-01-27 19:31         ` [alsa-devel] " Jyri Sarha
2014-01-28  9:23         ` Jean-Francois Moine
2014-01-28  9:23           ` Jean-Francois Moine
2014-01-30 12:20           ` Jyri Sarha
2014-01-30 12:20             ` [alsa-devel] " Jyri Sarha
2014-01-30 18:00             ` Jean-Francois Moine [this message]
2014-01-30 18:00               ` Jean-Francois Moine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140130190011.3849cf37@armhf \
    --to=moinejf@free.fr \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=detheridge@ti.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jsarha@ti.com \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=peter.ujfalusi@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.