linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] media: i2c: adv748x: Fix CBUS page issue
@ 2018-02-27 15:05 Kieran Bingham
  2018-02-27 15:05 ` [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration Kieran Bingham
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Kieran Bingham @ 2018-02-27 15:05 UTC (permalink / raw)
  To: linux-renesas-soc, linux-media, Kieran Bingham
  Cc: niklas.soderlund, Kieran Bingham

The ADV748x has 12 pages mapped on to I2C addresses.

The existing implementation only has 11 of these in the map enumeration, and
this can cause an off-by-one error when programming the map addresses.

This short series simplifies the regmap configuration tables, and adds the
missing CBUS page to better model the device, and remove the off by one error.

Once the tables are corrected, we add support for overriding the default map
addresses through the device tree using i2c_new_secondary_device().

Kieran Bingham (3):
  media: i2c: adv748x: Simplify regmap configuration
  media: i2c: adv748x: Add missing CBUS page
  media: i2c: adv748x: Add support for i2c_new_secondary_device

 drivers/media/i2c/adv748x/adv748x-core.c | 185 ++++++++++---------------------
 drivers/media/i2c/adv748x/adv748x.h      |  14 +--
 2 files changed, 58 insertions(+), 141 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration
  2018-02-27 15:05 [PATCH v2 0/3] media: i2c: adv748x: Fix CBUS page issue Kieran Bingham
@ 2018-02-27 15:05 ` Kieran Bingham
  2018-03-01 15:47   ` Niklas Söderlund
  2018-02-27 15:05 ` [PATCH v2 2/3] media: i2c: adv748x: Add missing CBUS page Kieran Bingham
  2018-02-27 15:05 ` [PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device Kieran Bingham
  2 siblings, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2018-02-27 15:05 UTC (permalink / raw)
  To: linux-renesas-soc, linux-media, Kieran Bingham
  Cc: niklas.soderlund, Mauro Carvalho Chehab, open list

From: Kieran Bingham <kieran.bingham@ideasonboard.com>

The ADV748x has identical map configurations for each register map. The
duplication of each map can be simplified using a helper macro such that
each map is represented on a single line.

Define ADV748X_REGMAP_CONF for this purpose use it to create the tables.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v2:
 - Remove unnecessary #undef

 drivers/media/i2c/adv748x/adv748x-core.c | 109 ++++++-------------------------
 1 file changed, 20 insertions(+), 89 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index fd92c9e4b519..faf73949962b 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -35,96 +35,27 @@
  * Register manipulation
  */
 
-static const struct regmap_config adv748x_regmap_cnf[] = {
-	{
-		.name			= "io",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "dpll",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "cp",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "hdmi",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "edid",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "repeater",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "infoframe",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "cec",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "sdp",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-
-	{
-		.name			= "txb",
-		.reg_bits		= 8,
-		.val_bits		= 8,
-
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
-	{
-		.name			= "txa",
-		.reg_bits		= 8,
-		.val_bits		= 8,
+#define ADV748X_REGMAP_CONF(n) \
+{ \
+	.name = n, \
+	.reg_bits = 8, \
+	.val_bits = 8, \
+	.max_register = 0xff, \
+	.cache_type = REGCACHE_NONE, \
+}
 
-		.max_register		= 0xff,
-		.cache_type		= REGCACHE_NONE,
-	},
+static const struct regmap_config adv748x_regmap_cnf[] = {
+	ADV748X_REGMAP_CONF("io"),
+	ADV748X_REGMAP_CONF("dpll"),
+	ADV748X_REGMAP_CONF("cp"),
+	ADV748X_REGMAP_CONF("hdmi"),
+	ADV748X_REGMAP_CONF("edid"),
+	ADV748X_REGMAP_CONF("repeater"),
+	ADV748X_REGMAP_CONF("infoframe"),
+	ADV748X_REGMAP_CONF("cec"),
+	ADV748X_REGMAP_CONF("sdp"),
+	ADV748X_REGMAP_CONF("txa"),
+	ADV748X_REGMAP_CONF("txb"),
 };
 
 static int adv748x_configure_regmap(struct adv748x_state *state, int region)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] media: i2c: adv748x: Add missing CBUS page
  2018-02-27 15:05 [PATCH v2 0/3] media: i2c: adv748x: Fix CBUS page issue Kieran Bingham
  2018-02-27 15:05 ` [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration Kieran Bingham
@ 2018-02-27 15:05 ` Kieran Bingham
  2018-02-27 15:05 ` [PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device Kieran Bingham
  2 siblings, 0 replies; 6+ messages in thread
From: Kieran Bingham @ 2018-02-27 15:05 UTC (permalink / raw)
  To: linux-renesas-soc, linux-media, Kieran Bingham
  Cc: niklas.soderlund, Mauro Carvalho Chehab, open list

From: Kieran Bingham <kieran.bingham@ideasonboard.com>

The ADV748x has 12 pages mapped onto I2C addresses.

In the existing implementation only 11 are mapped correctly in the page
enumerations, which causes an off-by-one fault on pages above the
infoframe definition due to a missing 'CBUS' page.

This causes the address for the CEC, SDP, TXA, and TXB to be incorrectly
programmed during the iterations in adv748x_initialise_clients().

Until now this has gone un-noticed due to the fact that following the
creation of the clients - the device is reset and the addresses are
reprogrammed in manually by the call to "adv748x_write_regs(state,
adv748x_set_slave_address);"

As part of moving to dynamic i2c address allocations repair this by
providing the missing CBUS page definition.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
v2
 - Remove period from commit title.
 - Collect Niklas' RB tag.


 drivers/media/i2c/adv748x/adv748x-core.c | 3 +++
 drivers/media/i2c/adv748x/adv748x.h      | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index faf73949962b..9dcaadaca217 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -52,6 +52,7 @@ static const struct regmap_config adv748x_regmap_cnf[] = {
 	ADV748X_REGMAP_CONF("edid"),
 	ADV748X_REGMAP_CONF("repeater"),
 	ADV748X_REGMAP_CONF("infoframe"),
+	ADV748X_REGMAP_CONF("cbus"),
 	ADV748X_REGMAP_CONF("cec"),
 	ADV748X_REGMAP_CONF("sdp"),
 	ADV748X_REGMAP_CONF("txa"),
@@ -89,6 +90,7 @@ static int adv748x_i2c_addresses[ADV748X_PAGE_MAX] = {
 	ADV748X_I2C_EDID,
 	ADV748X_I2C_REPEATER,
 	ADV748X_I2C_INFOFRAME,
+	ADV748X_I2C_CBUS,
 	ADV748X_I2C_CEC,
 	ADV748X_I2C_SDP,
 	ADV748X_I2C_TXB,
@@ -352,6 +354,7 @@ static const struct adv748x_reg_value adv748x_set_slave_address[] = {
 	{ADV748X_PAGE_IO, 0xf6, ADV748X_I2C_EDID << 1},
 	{ADV748X_PAGE_IO, 0xf7, ADV748X_I2C_REPEATER << 1},
 	{ADV748X_PAGE_IO, 0xf8, ADV748X_I2C_INFOFRAME << 1},
+	{ADV748X_PAGE_IO, 0xf9, ADV748X_I2C_CBUS << 1},
 	{ADV748X_PAGE_IO, 0xfa, ADV748X_I2C_CEC << 1},
 	{ADV748X_PAGE_IO, 0xfb, ADV748X_I2C_SDP << 1},
 	{ADV748X_PAGE_IO, 0xfc, ADV748X_I2C_TXB << 1},
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 6789e2f3bc8c..725662edc4b8 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -35,6 +35,7 @@
 #define ADV748X_I2C_EDID		0x36	/* EDID Map */
 #define ADV748X_I2C_REPEATER		0x32	/* HDMI RX Repeater Map */
 #define ADV748X_I2C_INFOFRAME		0x31	/* HDMI RX InfoFrame Map */
+#define ADV748X_I2C_CBUS		0x30	/* CBUS MHL Map */
 #define ADV748X_I2C_CEC			0x41	/* CEC Map */
 #define ADV748X_I2C_SDP			0x79	/* SDP Map */
 #define ADV748X_I2C_TXB			0x48	/* CSI-TXB Map */
@@ -48,6 +49,7 @@ enum adv748x_page {
 	ADV748X_PAGE_EDID,
 	ADV748X_PAGE_REPEATER,
 	ADV748X_PAGE_INFOFRAME,
+	ADV748X_PAGE_CBUS,
 	ADV748X_PAGE_CEC,
 	ADV748X_PAGE_SDP,
 	ADV748X_PAGE_TXB,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device
  2018-02-27 15:05 [PATCH v2 0/3] media: i2c: adv748x: Fix CBUS page issue Kieran Bingham
  2018-02-27 15:05 ` [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration Kieran Bingham
  2018-02-27 15:05 ` [PATCH v2 2/3] media: i2c: adv748x: Add missing CBUS page Kieran Bingham
@ 2018-02-27 15:05 ` Kieran Bingham
  2018-03-01 15:56   ` Niklas Söderlund
  2 siblings, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2018-02-27 15:05 UTC (permalink / raw)
  To: linux-renesas-soc, linux-media, Kieran Bingham
  Cc: niklas.soderlund, Mauro Carvalho Chehab, open list

From: Kieran Bingham <kieran.bingham@ideasonboard.com>

The ADV748x has twelve 256-byte maps that can be accessed via the main
I2C ports. Each map has it own I2C address and acts as a standard slave
device on the I2C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 drivers/media/i2c/adv748x/adv748x-core.c | 77 +++++++++++++++-----------------
 drivers/media/i2c/adv748x/adv748x.h      | 14 ------
 2 files changed, 36 insertions(+), 55 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
index 9dcaadaca217..911ccf6eb68c 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -80,21 +80,24 @@ static int adv748x_configure_regmap(struct adv748x_state *state, int region)
 
 	return 0;
 }
+struct adv748x_register_map {
+	const char *name;
+	u8 default_addr;
+};
 
-/* Default addresses for the I2C pages */
-static int adv748x_i2c_addresses[ADV748X_PAGE_MAX] = {
-	ADV748X_I2C_IO,
-	ADV748X_I2C_DPLL,
-	ADV748X_I2C_CP,
-	ADV748X_I2C_HDMI,
-	ADV748X_I2C_EDID,
-	ADV748X_I2C_REPEATER,
-	ADV748X_I2C_INFOFRAME,
-	ADV748X_I2C_CBUS,
-	ADV748X_I2C_CEC,
-	ADV748X_I2C_SDP,
-	ADV748X_I2C_TXB,
-	ADV748X_I2C_TXA,
+static const struct adv748x_register_map adv748x_default_addresses[] = {
+	[ADV748X_PAGE_IO] = { "main", 0x70 },
+	[ADV748X_PAGE_DPLL] = { "dpll", 0x26 },
+	[ADV748X_PAGE_CP] = { "cp", 0x22 },
+	[ADV748X_PAGE_HDMI] = { "hdmi", 0x34 },
+	[ADV748X_PAGE_EDID] = { "edid", 0x36 },
+	[ADV748X_PAGE_REPEATER] = { "repeater", 0x32 },
+	[ADV748X_PAGE_INFOFRAME] = { "infoframe", 0x31 },
+	[ADV748X_PAGE_CBUS] = { "cbus", 0x30 },
+	[ADV748X_PAGE_CEC] = { "cec", 0x41 },
+	[ADV748X_PAGE_SDP] = { "sdp", 0x79 },
+	[ADV748X_PAGE_TXB] = { "txb", 0x48 },
+	[ADV748X_PAGE_TXA] = { "txa", 0x4a },
 };
 
 static int adv748x_read_check(struct adv748x_state *state,
@@ -143,15 +146,20 @@ int adv748x_write_block(struct adv748x_state *state, int client_page,
 	return regmap_raw_write(regmap, init_reg, val, val_len);
 }
 
-static struct i2c_client *adv748x_dummy_client(struct adv748x_state *state,
-					       u8 addr, u8 io_reg)
+static int adv748x_set_slave_addresses(struct adv748x_state *state)
 {
-	struct i2c_client *client = state->client;
+	struct i2c_client *client;
+	unsigned int i;
+	u8 io_reg;
+
+	for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
+		io_reg = ADV748X_IO_SLAVE_ADDR_BASE + i;
+		client = state->i2c_clients[i];
 
-	if (addr)
-		io_write(state, io_reg, addr << 1);
+		io_write(state, io_reg, client->addr << 1);
+	}
 
-	return i2c_new_dummy(client->adapter, io_read(state, io_reg) >> 1);
+	return 0;
 }
 
 static void adv748x_unregister_clients(struct adv748x_state *state)
@@ -164,13 +172,15 @@ static void adv748x_unregister_clients(struct adv748x_state *state)
 
 static int adv748x_initialise_clients(struct adv748x_state *state)
 {
-	int i;
+	unsigned int i;
 	int ret;
 
 	for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
-		state->i2c_clients[i] =
-			adv748x_dummy_client(state, adv748x_i2c_addresses[i],
-					     ADV748X_IO_SLAVE_ADDR_BASE + i);
+		state->i2c_clients[i] = i2c_new_secondary_device(
+				state->client,
+				adv748x_default_addresses[i].name,
+				adv748x_default_addresses[i].default_addr);
+
 		if (state->i2c_clients[i] == NULL) {
 			adv_err(state, "failed to create i2c client %u\n", i);
 			return -ENOMEM;
@@ -181,7 +191,7 @@ static int adv748x_initialise_clients(struct adv748x_state *state)
 			return ret;
 	}
 
-	return 0;
+	return adv748x_set_slave_addresses(state);
 }
 
 /**
@@ -347,21 +357,6 @@ static const struct adv748x_reg_value adv748x_sw_reset[] = {
 	{ADV748X_PAGE_EOR, 0xff, 0xff}	/* End of register table */
 };
 
-static const struct adv748x_reg_value adv748x_set_slave_address[] = {
-	{ADV748X_PAGE_IO, 0xf3, ADV748X_I2C_DPLL << 1},
-	{ADV748X_PAGE_IO, 0xf4, ADV748X_I2C_CP << 1},
-	{ADV748X_PAGE_IO, 0xf5, ADV748X_I2C_HDMI << 1},
-	{ADV748X_PAGE_IO, 0xf6, ADV748X_I2C_EDID << 1},
-	{ADV748X_PAGE_IO, 0xf7, ADV748X_I2C_REPEATER << 1},
-	{ADV748X_PAGE_IO, 0xf8, ADV748X_I2C_INFOFRAME << 1},
-	{ADV748X_PAGE_IO, 0xf9, ADV748X_I2C_CBUS << 1},
-	{ADV748X_PAGE_IO, 0xfa, ADV748X_I2C_CEC << 1},
-	{ADV748X_PAGE_IO, 0xfb, ADV748X_I2C_SDP << 1},
-	{ADV748X_PAGE_IO, 0xfc, ADV748X_I2C_TXB << 1},
-	{ADV748X_PAGE_IO, 0xfd, ADV748X_I2C_TXA << 1},
-	{ADV748X_PAGE_EOR, 0xff, 0xff}	/* End of register table */
-};
-
 /* Supported Formats For Script Below */
 /* - 01-29 HDMI to MIPI TxA CSI 4-Lane - RGB888: */
 static const struct adv748x_reg_value adv748x_init_txa_4lane[] = {
@@ -492,7 +487,7 @@ static int adv748x_reset(struct adv748x_state *state)
 	if (ret < 0)
 		return ret;
 
-	ret = adv748x_write_regs(state, adv748x_set_slave_address);
+	ret = adv748x_set_slave_addresses(state);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
index 725662edc4b8..65f83741277e 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -27,20 +27,6 @@
 #ifndef _ADV748X_H_
 #define _ADV748X_H_
 
-/* I2C slave addresses */
-#define ADV748X_I2C_IO			0x70	/* IO Map */
-#define ADV748X_I2C_DPLL		0x26	/* DPLL Map */
-#define ADV748X_I2C_CP			0x22	/* CP Map */
-#define ADV748X_I2C_HDMI		0x34	/* HDMI Map */
-#define ADV748X_I2C_EDID		0x36	/* EDID Map */
-#define ADV748X_I2C_REPEATER		0x32	/* HDMI RX Repeater Map */
-#define ADV748X_I2C_INFOFRAME		0x31	/* HDMI RX InfoFrame Map */
-#define ADV748X_I2C_CBUS		0x30	/* CBUS MHL Map */
-#define ADV748X_I2C_CEC			0x41	/* CEC Map */
-#define ADV748X_I2C_SDP			0x79	/* SDP Map */
-#define ADV748X_I2C_TXB			0x48	/* CSI-TXB Map */
-#define ADV748X_I2C_TXA			0x4a	/* CSI-TXA Map */
-
 enum adv748x_page {
 	ADV748X_PAGE_IO,
 	ADV748X_PAGE_DPLL,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration
  2018-02-27 15:05 ` [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration Kieran Bingham
@ 2018-03-01 15:47   ` Niklas Söderlund
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2018-03-01 15:47 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: linux-renesas-soc, linux-media, Kieran Bingham,
	Mauro Carvalho Chehab, open list

Hi Kieran,

Thanks for your patch,

On 2018-02-27 15:05:48 +0000, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> The ADV748x has identical map configurations for each register map. The
> duplication of each map can be simplified using a helper macro such that
> each map is represented on a single line.
> 
> Define ADV748X_REGMAP_CONF for this purpose use it to create the tables.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> ---
> v2:
>  - Remove unnecessary #undef
> 
>  drivers/media/i2c/adv748x/adv748x-core.c | 109 ++++++-------------------------
>  1 file changed, 20 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
> index fd92c9e4b519..faf73949962b 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -35,96 +35,27 @@
>   * Register manipulation
>   */
>  
> -static const struct regmap_config adv748x_regmap_cnf[] = {
> -	{
> -		.name			= "io",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "dpll",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "cp",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "hdmi",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "edid",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "repeater",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "infoframe",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "cec",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "sdp",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -
> -	{
> -		.name			= "txb",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> -
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> -	{
> -		.name			= "txa",
> -		.reg_bits		= 8,
> -		.val_bits		= 8,
> +#define ADV748X_REGMAP_CONF(n) \
> +{ \
> +	.name = n, \
> +	.reg_bits = 8, \
> +	.val_bits = 8, \
> +	.max_register = 0xff, \
> +	.cache_type = REGCACHE_NONE, \
> +}
>  
> -		.max_register		= 0xff,
> -		.cache_type		= REGCACHE_NONE,
> -	},
> +static const struct regmap_config adv748x_regmap_cnf[] = {
> +	ADV748X_REGMAP_CONF("io"),
> +	ADV748X_REGMAP_CONF("dpll"),
> +	ADV748X_REGMAP_CONF("cp"),
> +	ADV748X_REGMAP_CONF("hdmi"),
> +	ADV748X_REGMAP_CONF("edid"),
> +	ADV748X_REGMAP_CONF("repeater"),
> +	ADV748X_REGMAP_CONF("infoframe"),
> +	ADV748X_REGMAP_CONF("cec"),
> +	ADV748X_REGMAP_CONF("sdp"),
> +	ADV748X_REGMAP_CONF("txa"),
> +	ADV748X_REGMAP_CONF("txb"),
>  };
>  
>  static int adv748x_configure_regmap(struct adv748x_state *state, int region)
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device
  2018-02-27 15:05 ` [PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device Kieran Bingham
@ 2018-03-01 15:56   ` Niklas Söderlund
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2018-03-01 15:56 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: linux-renesas-soc, linux-media, Kieran Bingham,
	Mauro Carvalho Chehab, open list

Hi Kieran,

I like this change :-)

On 2018-02-27 15:05:50 +0000, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> The ADV748x has twelve 256-byte maps that can be accessed via the main
> I2C ports. Each map has it own I2C address and acts as a standard slave
> device on the I2C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/i2c/adv748x/adv748x-core.c | 77 +++++++++++++++-----------------
>  drivers/media/i2c/adv748x/adv748x.h      | 14 ------
>  2 files changed, 36 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
> index 9dcaadaca217..911ccf6eb68c 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -80,21 +80,24 @@ static int adv748x_configure_regmap(struct adv748x_state *state, int region)
>  
>  	return 0;
>  }
> +struct adv748x_register_map {
> +	const char *name;
> +	u8 default_addr;
> +};
>  
> -/* Default addresses for the I2C pages */
> -static int adv748x_i2c_addresses[ADV748X_PAGE_MAX] = {
> -	ADV748X_I2C_IO,
> -	ADV748X_I2C_DPLL,
> -	ADV748X_I2C_CP,
> -	ADV748X_I2C_HDMI,
> -	ADV748X_I2C_EDID,
> -	ADV748X_I2C_REPEATER,
> -	ADV748X_I2C_INFOFRAME,
> -	ADV748X_I2C_CBUS,
> -	ADV748X_I2C_CEC,
> -	ADV748X_I2C_SDP,
> -	ADV748X_I2C_TXB,
> -	ADV748X_I2C_TXA,
> +static const struct adv748x_register_map adv748x_default_addresses[] = {
> +	[ADV748X_PAGE_IO] = { "main", 0x70 },
> +	[ADV748X_PAGE_DPLL] = { "dpll", 0x26 },
> +	[ADV748X_PAGE_CP] = { "cp", 0x22 },
> +	[ADV748X_PAGE_HDMI] = { "hdmi", 0x34 },
> +	[ADV748X_PAGE_EDID] = { "edid", 0x36 },
> +	[ADV748X_PAGE_REPEATER] = { "repeater", 0x32 },
> +	[ADV748X_PAGE_INFOFRAME] = { "infoframe", 0x31 },
> +	[ADV748X_PAGE_CBUS] = { "cbus", 0x30 },
> +	[ADV748X_PAGE_CEC] = { "cec", 0x41 },
> +	[ADV748X_PAGE_SDP] = { "sdp", 0x79 },
> +	[ADV748X_PAGE_TXB] = { "txb", 0x48 },
> +	[ADV748X_PAGE_TXA] = { "txa", 0x4a },
>  };
>  
>  static int adv748x_read_check(struct adv748x_state *state,
> @@ -143,15 +146,20 @@ int adv748x_write_block(struct adv748x_state *state, int client_page,
>  	return regmap_raw_write(regmap, init_reg, val, val_len);
>  }
>  
> -static struct i2c_client *adv748x_dummy_client(struct adv748x_state *state,
> -					       u8 addr, u8 io_reg)
> +static int adv748x_set_slave_addresses(struct adv748x_state *state)
>  {
> -	struct i2c_client *client = state->client;
> +	struct i2c_client *client;
> +	unsigned int i;
> +	u8 io_reg;
> +
> +	for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
> +		io_reg = ADV748X_IO_SLAVE_ADDR_BASE + i;
> +		client = state->i2c_clients[i];
>  
> -	if (addr)
> -		io_write(state, io_reg, addr << 1);
> +		io_write(state, io_reg, client->addr << 1);
> +	}
>  
> -	return i2c_new_dummy(client->adapter, io_read(state, io_reg) >> 1);
> +	return 0;
>  }
>  
>  static void adv748x_unregister_clients(struct adv748x_state *state)
> @@ -164,13 +172,15 @@ static void adv748x_unregister_clients(struct adv748x_state *state)
>  
>  static int adv748x_initialise_clients(struct adv748x_state *state)
>  {
> -	int i;
> +	unsigned int i;
>  	int ret;
>  
>  	for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
> -		state->i2c_clients[i] =
> -			adv748x_dummy_client(state, adv748x_i2c_addresses[i],
> -					     ADV748X_IO_SLAVE_ADDR_BASE + i);
> +		state->i2c_clients[i] = i2c_new_secondary_device(
> +				state->client,
> +				adv748x_default_addresses[i].name,
> +				adv748x_default_addresses[i].default_addr);
> +
>  		if (state->i2c_clients[i] == NULL) {
>  			adv_err(state, "failed to create i2c client %u\n", i);
>  			return -ENOMEM;
> @@ -181,7 +191,7 @@ static int adv748x_initialise_clients(struct adv748x_state *state)
>  			return ret;
>  	}
>  
> -	return 0;
> +	return adv748x_set_slave_addresses(state);
>  }
>  
>  /**
> @@ -347,21 +357,6 @@ static const struct adv748x_reg_value adv748x_sw_reset[] = {
>  	{ADV748X_PAGE_EOR, 0xff, 0xff}	/* End of register table */
>  };
>  
> -static const struct adv748x_reg_value adv748x_set_slave_address[] = {
> -	{ADV748X_PAGE_IO, 0xf3, ADV748X_I2C_DPLL << 1},
> -	{ADV748X_PAGE_IO, 0xf4, ADV748X_I2C_CP << 1},
> -	{ADV748X_PAGE_IO, 0xf5, ADV748X_I2C_HDMI << 1},
> -	{ADV748X_PAGE_IO, 0xf6, ADV748X_I2C_EDID << 1},
> -	{ADV748X_PAGE_IO, 0xf7, ADV748X_I2C_REPEATER << 1},
> -	{ADV748X_PAGE_IO, 0xf8, ADV748X_I2C_INFOFRAME << 1},
> -	{ADV748X_PAGE_IO, 0xf9, ADV748X_I2C_CBUS << 1},
> -	{ADV748X_PAGE_IO, 0xfa, ADV748X_I2C_CEC << 1},
> -	{ADV748X_PAGE_IO, 0xfb, ADV748X_I2C_SDP << 1},
> -	{ADV748X_PAGE_IO, 0xfc, ADV748X_I2C_TXB << 1},
> -	{ADV748X_PAGE_IO, 0xfd, ADV748X_I2C_TXA << 1},
> -	{ADV748X_PAGE_EOR, 0xff, 0xff}	/* End of register table */
> -};
> -
>  /* Supported Formats For Script Below */
>  /* - 01-29 HDMI to MIPI TxA CSI 4-Lane - RGB888: */
>  static const struct adv748x_reg_value adv748x_init_txa_4lane[] = {
> @@ -492,7 +487,7 @@ static int adv748x_reset(struct adv748x_state *state)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = adv748x_write_regs(state, adv748x_set_slave_address);
> +	ret = adv748x_set_slave_addresses(state);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h
> index 725662edc4b8..65f83741277e 100644
> --- a/drivers/media/i2c/adv748x/adv748x.h
> +++ b/drivers/media/i2c/adv748x/adv748x.h
> @@ -27,20 +27,6 @@
>  #ifndef _ADV748X_H_
>  #define _ADV748X_H_
>  
> -/* I2C slave addresses */
> -#define ADV748X_I2C_IO			0x70	/* IO Map */
> -#define ADV748X_I2C_DPLL		0x26	/* DPLL Map */
> -#define ADV748X_I2C_CP			0x22	/* CP Map */
> -#define ADV748X_I2C_HDMI		0x34	/* HDMI Map */
> -#define ADV748X_I2C_EDID		0x36	/* EDID Map */
> -#define ADV748X_I2C_REPEATER		0x32	/* HDMI RX Repeater Map */
> -#define ADV748X_I2C_INFOFRAME		0x31	/* HDMI RX InfoFrame Map */
> -#define ADV748X_I2C_CBUS		0x30	/* CBUS MHL Map */
> -#define ADV748X_I2C_CEC			0x41	/* CEC Map */
> -#define ADV748X_I2C_SDP			0x79	/* SDP Map */
> -#define ADV748X_I2C_TXB			0x48	/* CSI-TXB Map */
> -#define ADV748X_I2C_TXA			0x4a	/* CSI-TXA Map */
> -
>  enum adv748x_page {
>  	ADV748X_PAGE_IO,
>  	ADV748X_PAGE_DPLL,
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-01 15:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-27 15:05 [PATCH v2 0/3] media: i2c: adv748x: Fix CBUS page issue Kieran Bingham
2018-02-27 15:05 ` [PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration Kieran Bingham
2018-03-01 15:47   ` Niklas Söderlund
2018-02-27 15:05 ` [PATCH v2 2/3] media: i2c: adv748x: Add missing CBUS page Kieran Bingham
2018-02-27 15:05 ` [PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device Kieran Bingham
2018-03-01 15:56   ` Niklas Söderlund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).