Re: [PATCH v2 2/2] adv7604: Use DT parsing in dummy creation

[Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>]

On 08/29/2014 05:15 PM, Jean-Michel Hautbois wrote:
> This patch uses DT in order to parse addresses for dummy devices of adv7604.
> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> I²C ports. Each map has it own I²C address and acts
> as a standard slave device on the I²C bus.
>
> If nothing is defined, it uses default addresses.
> The main prupose is using two adv76xx on the same i2c bus.

Ideally this patch is split up in two patches. One patch adding support for 
i2c_new_secondary_device() and one patch adding support for DT for the adv7604.

[...]
> +static const char const *adv7604_secondary_names[] = {
> +	"main", /* ADV7604_PAGE_IO */

How about [ADV7604_PAGE_IO] = "main", instead of having the comment, this 
makes things more explicit.

> +	"avlink", /* ADV7604_PAGE_AVLINK */
> +	"cec", /* ADV7604_PAGE_CEC */
> +	"infoframe", /* ADV7604_PAGE_INFOFRAME */
> +	"esdp", /* ADV7604_PAGE_ESDP */
> +	"dpp", /* ADV7604_PAGE_DPP */
> +	"afe", /* ADV7604_PAGE_AFE */
> +	"rep", /* ADV7604_PAGE_REP */
> +	"edid", /* ADV7604_PAGE_EDID */
> +	"hdmi", /* ADV7604_PAGE_HDMI */
> +	"test", /* ADV7604_PAGE_TEST */
> +	"cp", /* ADV7604_PAGE_CP */
> +	"vdp" /* ADV7604_PAGE_VDP */
> +};
> +
>   /* ----------------------------------------------------------------------- */
>
>   static inline struct adv7604_state *to_state(struct v4l2_subdev *sd)
> @@ -2528,13 +2544,31 @@ static void adv7604_unregister_clients(struct adv7604_state *state)
>   }
>
>   static struct i2c_client *adv7604_dummy_client(struct v4l2_subdev *sd,
> -							u8 addr, u8 io_reg)
> +						unsigned int i)
>   {
>   	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +	struct adv7604_platform_data *pdata = client->dev.platform_data;
> +	unsigned int io_reg = 0xf2 + i;
> +	unsigned int default_addr = io_read(sd, io_reg) >> 1;
> +	struct i2c_client *new_client;
> +
> +	if (IS_ENABLED(CONFIG_OF)) {

No CONFIG_OF. i2c_new_secondary_device() is supposed to be the generic 
method of instantiating the secondary i2c_client, regardless of how the 
address is specified. For this driver we still need to keep the old method 
of instantiation via platform data for legacy reasons for now. So what this 
should look like is:


	if (pdata && pdata->i2c_addresses[i])
		new_client = i2c_new_dummy(client->adapter,
			pdata->i2c_addresses[i]);	
	else
		new_client = i2c_new_secondary_device(client,
			adv7604_secondary_names[i], default_addr);


> +		/* Try to find it in DT */
> +		new_client = i2c_new_secondary_device(client,
> +			adv7604_secondary_names[i], default_addr);
> +	} else if (pdata) {
> +		if (pdata->i2c_addresses[i])
> +			new_client = i2c_new_dummy(client->adapter,
> +						pdata->i2c_addresses[i]);
> +		else
> +			new_client = i2c_new_dummy(client->adapter,
> +						default_addr);
> +	}
>
[...]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html