[PATCH v2] media: i2c: ADV7604: Migrate to regmap

2015-06-16 Thread Pablo Anton
This is a preliminary patch in order to add support for ALSA.
It replaces all current i2c access with regmap.

Signed-off-by: Pablo Anton 
Signed-off-by: Jean-Michel Hautbois 
---
v2: Rebase after renaming

 drivers/media/i2c/adv7604.c | 337 
 1 file changed, 244 insertions(+), 93 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 60ffcf0..38ae454 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -166,6 +167,9 @@ struct adv76xx_state {
/* i2c clients */
struct i2c_client *i2c_clients[ADV76XX_PAGE_MAX];
 
+   /* Regmaps */
+   struct regmap *regmap[ADV76XX_PAGE_MAX];
+
/* controls */
struct v4l2_ctrl *detect_tx_5v_ctrl;
struct v4l2_ctrl *analog_sampling_phase_ctrl;
@@ -346,66 +350,39 @@ static inline unsigned vtotal(const struct 
v4l2_bt_timings *t)
 
 /* --- */
 
-static s32 adv_smbus_read_byte_data_check(struct i2c_client *client,
-   u8 command, bool check)
-{
-   union i2c_smbus_data data;
-
-   if (!i2c_smbus_xfer(client->adapter, client->addr, client->flags,
-   I2C_SMBUS_READ, command,
-   I2C_SMBUS_BYTE_DATA, &data))
-   return data.byte;
-   if (check)
-   v4l_err(client, "error reading %02x, %02x\n",
-   client->addr, command);
-   return -EIO;
-}
-
-static s32 adv_smbus_read_byte_data(struct adv76xx_state *state,
-   enum adv76xx_page page, u8 command)
+static int adv76xx_read_check(struct adv76xx_state *state,
+int client_page, u8 reg)
 {
-   return adv_smbus_read_byte_data_check(state->i2c_clients[page],
- command, true);
-}
-
-static s32 adv_smbus_write_byte_data(struct adv76xx_state *state,
-enum adv76xx_page page, u8 command,
-u8 value)
-{
-   struct i2c_client *client = state->i2c_clients[page];
-   union i2c_smbus_data data;
+   struct i2c_client *client = state->i2c_clients[client_page];
int err;
-   int i;
+   unsigned int val;
 
-   data.byte = value;
-   for (i = 0; i < 3; i++) {
-   err = i2c_smbus_xfer(client->adapter, client->addr,
-   client->flags,
-   I2C_SMBUS_WRITE, command,
-   I2C_SMBUS_BYTE_DATA, &data);
-   if (!err)
-   break;
+   err = regmap_read(state->regmap[client_page], reg, &val);
+
+   if (err) {
+   v4l_err(client, "error reading %02x, %02x\n",
+   client->addr, reg);
+   return err;
}
-   if (err < 0)
-   v4l_err(client, "error writing %02x, %02x, %02x\n",
-   client->addr, command, value);
-   return err;
+   return val;
 }
 
-static s32 adv_smbus_write_i2c_block_data(struct adv76xx_state *state,
- enum adv76xx_page page, u8 command,
- unsigned length, const u8 *values)
+/* adv76xx_write_block(): Write raw data with a maximum of I2C_SMBUS_BLOCK_MAX
+ * size to one or more registers.
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+static int adv76xx_write_block(struct adv76xx_state *state, int client_page,
+ unsigned int init_reg, const void *val,
+ size_t val_len)
 {
-   struct i2c_client *client = state->i2c_clients[page];
-   union i2c_smbus_data data;
+   struct regmap *regmap = state->regmap[client_page];
+
+   if (val_len > I2C_SMBUS_BLOCK_MAX)
+   val_len = I2C_SMBUS_BLOCK_MAX;
 
-   if (length > I2C_SMBUS_BLOCK_MAX)
-   length = I2C_SMBUS_BLOCK_MAX;
-   data.block[0] = length;
-   memcpy(data.block + 1, values, length);
-   return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_WRITE, command,
- I2C_SMBUS_I2C_BLOCK_DATA, &data);
+   return regmap_raw_write(regmap, init_reg, val, val_len);
 }
 
 /* --- */
@@ -414,14 +391,14 @@ static inline int io_read(struct v4l2_subdev *sd, u8 reg)
 {
struct adv76xx_state *state = to_state(sd);
 
-   return adv_smbus_read_byte_data(state, ADV76XX_PAGE_IO, reg);
+   return adv76xx_read_check(state, ADV76XX_PAGE_IO, reg);
 }
 
 static inline int io_write(struct v4l2_subdev *sd, u8 reg, u8 val)
 {
struct adv76xx_state

Re: [PATCH v2] media: i2c: ADV7604: Migrate to regmap

2015-06-19 Thread Lars-Peter Clausen

On 06/16/2015 10:38 AM, Pablo Anton wrote:

This is a preliminary patch in order to add support for ALSA.
It replaces all current i2c access with regmap.


Looks pretty good.


  #define ADV76XX_REG(page, offset) (((page) << 8) | (offset))
@@ -633,13 +618,15 @@ static int adv76xx_read_reg(struct v4l2_subdev *sd, 
unsigned int reg)
  {
struct adv76xx_state *state = to_state(sd);
unsigned int page = reg >> 8;
+   unsigned int val;

if (!(BIT(page) & state->info->page_mask))
return -EINVAL;

reg &= 0xff;
+   regmap_read(state->regmap[page], reg, &val);


should check return value of regmap_read.



-   return adv_smbus_read_byte_data(state, page, reg);
+   return val;
  }
  #endif



+static int configure_regmap(struct adv76xx_state *state, int region)
+{
+   int err;
+
+   if (!state->i2c_clients[region])
+   return -ENODEV;
+
+   if (!state->regmap[region]) {
+
+   state->regmap[region] =
+   devm_regmap_init_i2c(state->i2c_clients[region],
+&adv76xx_regmap_cnf[region]);
+
+   if (IS_ERR(state->regmap[region])) {
+   err = PTR_ERR(state->regmap[region]);
+   v4l_err(state->i2c_clients[region],
+   "Error initializing regmap %d with error 
%d\n",
+   region, err);
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
+static int configure_regmaps(struct adv76xx_state *state)
+{
+   int i, err;
+
+   for (i = 0 ; i < ADV76XX_PAGE_MAX; i++) {


The IO page was already initilaized earlier on, so this should start with i 
= ADV7604_PAGE_AVLINK.



+   err = configure_regmap(state, i);
+   if (err && (err != -ENODEV))
+   return err;
+   }
+   return 0;
+}
+
  static int adv76xx_probe(struct i2c_client *client,
 const struct i2c_device_id *id)
  {
@@ -2683,7 +2815,7 @@ static int adv76xx_probe(struct i2c_client *client,
struct v4l2_ctrl_handler *hdl;
struct v4l2_subdev *sd;
unsigned int i;
-   u16 val;
+   unsigned int val, val2;
int err;

/* Check if the adapter supports the needed features */
@@ -2747,22 +2879,36 @@ static int adv76xx_probe(struct i2c_client *client,
client->addr);
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;

+   /* Configure IO Regmap region */
+   err = configure_regmap(state, ADV76XX_PAGE_IO);
+
+   if (err) {
+   v4l2_info(sd, "Error configuring IO regmap region\n");
+   return -ENODEV;
+   }
+
/*
 * Verify that the chip is present. On ADV7604 the RD_INFO register only
 * identifies the revision, while on ADV7611 it identifies the model as
 * well. Use the HDMI slave address on ADV7604 and RD_INFO on ADV7611.
 */
if (state->info->type == ADV7604) {
-   val = adv_smbus_read_byte_data_check(client, 0xfb, false);
+   regmap_read(state->regmap[ADV76XX_PAGE_IO], 0xfb, &val);
if (val != 0x68) {
v4l2_info(sd, "not an adv7604 on address 0x%x\n",
client->addr << 1);
return -ENODEV;
}
} else {
-   val = (adv_smbus_read_byte_data_check(client, 0xea, false) << 8)
-   | (adv_smbus_read_byte_data_check(client, 0xeb, false) << 
0);
-   if (val != 0x2051) {
+   regmap_read(state->regmap[ADV76XX_PAGE_IO],
+   0xea,
+   &val);
+   val2 = val << 8;
+   regmap_read(state->regmap[ADV76XX_PAGE_IO],
+   0xeb,
+   &val);


we should check the return value of regmap_read to make sure the device 
responds.



+   val2 |= val;
+   if (val2 != 0x2051) {
v4l2_info(sd, "not an adv7611 on address 0x%x\n",
client->addr << 1);
return -ENODEV;
@@ -2853,6 +2999,11 @@ static int adv76xx_probe(struct i2c_client *client,
if (err)
goto err_work_queues;

+   /* Configure regmaps */
+   err = configure_regmaps(state);
+   if (err)
+   goto err_entity;
+
err = adv76xx_core_init(sd);
if (err)
goto err_entity;



--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] media: i2c: ADV7604: Migrate to regmap

2015-06-19 Thread Hans Verkuil
On 06/16/2015 10:38 AM, Pablo Anton wrote:
> This is a preliminary patch in order to add support for ALSA.
> It replaces all current i2c access with regmap.
> 
> Signed-off-by: Pablo Anton 
> Signed-off-by: Jean-Michel Hautbois 

After fixing the missing return value check that Lars-Peter found you can
add my:

Acked-by: Hans Verkuil 
Tested-by: Hans Verkuil 

Regards,

Hans

> ---
> v2: Rebase after renaming
> 
>  drivers/media/i2c/adv7604.c | 337 
> 
>  1 file changed, 244 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 60ffcf0..38ae454 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -166,6 +167,9 @@ struct adv76xx_state {
>   /* i2c clients */
>   struct i2c_client *i2c_clients[ADV76XX_PAGE_MAX];
>  
> + /* Regmaps */
> + struct regmap *regmap[ADV76XX_PAGE_MAX];
> +
>   /* controls */
>   struct v4l2_ctrl *detect_tx_5v_ctrl;
>   struct v4l2_ctrl *analog_sampling_phase_ctrl;
> @@ -346,66 +350,39 @@ static inline unsigned vtotal(const struct 
> v4l2_bt_timings *t)
>  
>  /* --- */
>  
> -static s32 adv_smbus_read_byte_data_check(struct i2c_client *client,
> - u8 command, bool check)
> -{
> - union i2c_smbus_data data;
> -
> - if (!i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> - I2C_SMBUS_READ, command,
> - I2C_SMBUS_BYTE_DATA, &data))
> - return data.byte;
> - if (check)
> - v4l_err(client, "error reading %02x, %02x\n",
> - client->addr, command);
> - return -EIO;
> -}
> -
> -static s32 adv_smbus_read_byte_data(struct adv76xx_state *state,
> - enum adv76xx_page page, u8 command)
> +static int adv76xx_read_check(struct adv76xx_state *state,
> +  int client_page, u8 reg)
>  {
> - return adv_smbus_read_byte_data_check(state->i2c_clients[page],
> -   command, true);
> -}
> -
> -static s32 adv_smbus_write_byte_data(struct adv76xx_state *state,
> -  enum adv76xx_page page, u8 command,
> -  u8 value)
> -{
> - struct i2c_client *client = state->i2c_clients[page];
> - union i2c_smbus_data data;
> + struct i2c_client *client = state->i2c_clients[client_page];
>   int err;
> - int i;
> + unsigned int val;
>  
> - data.byte = value;
> - for (i = 0; i < 3; i++) {
> - err = i2c_smbus_xfer(client->adapter, client->addr,
> - client->flags,
> - I2C_SMBUS_WRITE, command,
> - I2C_SMBUS_BYTE_DATA, &data);
> - if (!err)
> - break;
> + err = regmap_read(state->regmap[client_page], reg, &val);
> +
> + if (err) {
> + v4l_err(client, "error reading %02x, %02x\n",
> + client->addr, reg);
> + return err;
>   }
> - if (err < 0)
> - v4l_err(client, "error writing %02x, %02x, %02x\n",
> - client->addr, command, value);
> - return err;
> + return val;
>  }
>  
> -static s32 adv_smbus_write_i2c_block_data(struct adv76xx_state *state,
> -   enum adv76xx_page page, u8 command,
> -   unsigned length, const u8 *values)
> +/* adv76xx_write_block(): Write raw data with a maximum of 
> I2C_SMBUS_BLOCK_MAX
> + * size to one or more registers.
> + *
> + * A value of zero will be returned on success, a negative errno will
> + * be returned in error cases.
> + */
> +static int adv76xx_write_block(struct adv76xx_state *state, int client_page,
> +   unsigned int init_reg, const void *val,
> +   size_t val_len)
>  {
> - struct i2c_client *client = state->i2c_clients[page];
> - union i2c_smbus_data data;
> + struct regmap *regmap = state->regmap[client_page];
> +
> + if (val_len > I2C_SMBUS_BLOCK_MAX)
> + val_len = I2C_SMBUS_BLOCK_MAX;
>  
> - if (length > I2C_SMBUS_BLOCK_MAX)
> - length = I2C_SMBUS_BLOCK_MAX;
> - data.block[0] = length;
> - memcpy(data.block + 1, values, length);
> - return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> -   I2C_SMBUS_WRITE, command,
> -   I2C_SMBUS_I2C_BLOCK_DATA, &data);
> + return regmap_raw_write(regmap, init_reg, val, val_len);
>  }
>  
>  /* --- */
> @@ -414,14 +391,14 @@ static inline int io_r