[PATCH v1 1/2] drm/i2c: tda998x: access chip registers via a regmap
Sven Van Asbroeck
thesven73 at gmail.com
Mon May 27 19:15:51 UTC 2019
The tda988x i2c chip registers are accessed through a
paged register scheme. The kernel regmap abstraction
supports paged accesses. Replace this driver's
dedicated i2c access functions with a standard i2c
regmap.
Pros:
- dedicated i2c access code disappears: accesses now
go through well-maintained regmap code
- page atomicity requirements now handled by regmap
- ro/wo/rw access modes are now explicitly defined:
any inappropriate register accesses (e.g. write to a
read-only register) will now be explicitly rejected
by the regmap core
- all tda988x registers are now visible via debugfs
Cons:
- this will probably require extensive testing
Tested on a TDA19988 using a Freescale/NXP imx6q.
Signed-off-by: Sven Van Asbroeck <TheSven73 at gmail.com>
---
I originally hacked this together while debugging an incompatibility between
the tda988x's audio input and the audio codec I was driving it with.
That required me to have debug access to the chip's register values.
A regmap did the trick, it has good debugfs support.
drivers/gpu/drm/i2c/tda998x_drv.c | 350 ++++++++++++++++++++----------
1 file changed, 234 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb515..8153e2e19e18 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -21,6 +21,7 @@
#include <linux/module.h>
#include <linux/platform_data/tda9950.h>
#include <linux/irq.h>
+#include <linux/regmap.h>
#include <sound/asoundef.h>
#include <sound/hdmi-codec.h>
@@ -43,10 +44,9 @@ struct tda998x_audio_port {
struct tda998x_priv {
struct i2c_client *cec;
struct i2c_client *hdmi;
- struct mutex mutex;
+ struct regmap *regmap;
u16 rev;
u8 cec_addr;
- u8 current_page;
bool is_on;
bool supports_infoframes;
bool sink_has_audio;
@@ -88,12 +88,10 @@ struct tda998x_priv {
/* The TDA9988 series of devices use a paged register scheme.. to simplify
* things we encode the page # in upper bits of the register #. To read/
* write a given register, we need to make sure CURPAGE register is set
- * appropriately. Which implies reads/writes are not atomic. Fun!
+ * appropriately.
*/
#define REG(page, addr) (((page) << 8) | (addr))
-#define REG2ADDR(reg) ((reg) & 0xff)
-#define REG2PAGE(reg) (((reg) >> 8) & 0xff)
#define REG_CURPAGE 0xff /* write */
@@ -285,8 +283,9 @@ struct tda998x_priv {
/* Page 09h: EDID Control */
+/* EDID_DATA consists of 128 successive registers read */
#define REG_EDID_DATA_0 REG(0x09, 0x00) /* read */
-/* next 127 successive registers are the EDID block */
+#define REG_EDID_DATA_127 REG(0x09, 0x7f) /* read */
#define REG_EDID_CTRL REG(0x09, 0xfa) /* read/write */
#define REG_DDC_ADDR REG(0x09, 0xfb) /* read/write */
#define REG_DDC_OFFS REG(0x09, 0xfc) /* read/write */
@@ -295,11 +294,17 @@ struct tda998x_priv {
/* Page 10h: information frames and packets */
+/* REG_IF1_HB consists of 32 successive registers read/write */
#define REG_IF1_HB0 REG(0x10, 0x20) /* read/write */
+/* REG_IF2_HB consists of 32 successive registers read/write */
#define REG_IF2_HB0 REG(0x10, 0x40) /* read/write */
+/* REG_IF3_HB consists of 32 successive registers read/write */
#define REG_IF3_HB0 REG(0x10, 0x60) /* read/write */
+/* REG_IF4_HB consists of 32 successive registers read/write */
#define REG_IF4_HB0 REG(0x10, 0x80) /* read/write */
+/* REG_IF5_HB consists of 32 successive registers read/write */
#define REG_IF5_HB0 REG(0x10, 0xa0) /* read/write */
+#define REG_IF5_HB31 REG(0x10, 0xbf) /* read/write */
/* Page 11h: audio settings and content info packets */
@@ -542,93 +547,29 @@ static void tda998x_cec_hook_release(void *data)
cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, false);
}
-static int
-set_page(struct tda998x_priv *priv, u16 reg)
-{
- if (REG2PAGE(reg) != priv->current_page) {
- struct i2c_client *client = priv->hdmi;
- u8 buf[] = {
- REG_CURPAGE, REG2PAGE(reg)
- };
- int ret = i2c_master_send(client, buf, sizeof(buf));
- if (ret < 0) {
- dev_err(&client->dev, "%s %04x err %d\n", __func__,
- reg, ret);
- return ret;
- }
-
- priv->current_page = REG2PAGE(reg);
- }
- return 0;
-}
-
static int
reg_read_range(struct tda998x_priv *priv, u16 reg, char *buf, int cnt)
{
- struct i2c_client *client = priv->hdmi;
- u8 addr = REG2ADDR(reg);
int ret;
- mutex_lock(&priv->mutex);
- ret = set_page(priv, reg);
- if (ret < 0)
- goto out;
-
- ret = i2c_master_send(client, &addr, sizeof(addr));
+ ret = regmap_bulk_read(priv->regmap, reg, buf, cnt);
if (ret < 0)
- goto fail;
-
- ret = i2c_master_recv(client, buf, cnt);
- if (ret < 0)
- goto fail;
-
- goto out;
-
-fail:
- dev_err(&client->dev, "Error %d reading from 0x%x\n", ret, reg);
-out:
- mutex_unlock(&priv->mutex);
- return ret;
+ return ret;
+ return cnt;
}
-#define MAX_WRITE_RANGE_BUF 32
-
static void
reg_write_range(struct tda998x_priv *priv, u16 reg, u8 *p, int cnt)
{
- struct i2c_client *client = priv->hdmi;
- /* This is the maximum size of the buffer passed in */
- u8 buf[MAX_WRITE_RANGE_BUF + 1];
- int ret;
-
- if (cnt > MAX_WRITE_RANGE_BUF) {
- dev_err(&client->dev, "Fixed write buffer too small (%d)\n",
- MAX_WRITE_RANGE_BUF);
- return;
- }
-
- buf[0] = REG2ADDR(reg);
- memcpy(&buf[1], p, cnt);
-
- mutex_lock(&priv->mutex);
- ret = set_page(priv, reg);
- if (ret < 0)
- goto out;
-
- ret = i2c_master_send(client, buf, cnt + 1);
- if (ret < 0)
- dev_err(&client->dev, "Error %d writing to 0x%x\n", ret, reg);
-out:
- mutex_unlock(&priv->mutex);
+ regmap_bulk_write(priv->regmap, reg, p, cnt);
}
static int
reg_read(struct tda998x_priv *priv, u16 reg)
{
- u8 val = 0;
- int ret;
+ int ret, val;
- ret = reg_read_range(priv, reg, &val, sizeof(val));
+ ret = regmap_read(priv->regmap, reg, &val);
if (ret < 0)
return ret;
return val;
@@ -637,59 +578,27 @@ reg_read(struct tda998x_priv *priv, u16 reg)
static void
reg_write(struct tda998x_priv *priv, u16 reg, u8 val)
{
- struct i2c_client *client = priv->hdmi;
- u8 buf[] = {REG2ADDR(reg), val};
- int ret;
-
- mutex_lock(&priv->mutex);
- ret = set_page(priv, reg);
- if (ret < 0)
- goto out;
-
- ret = i2c_master_send(client, buf, sizeof(buf));
- if (ret < 0)
- dev_err(&client->dev, "Error %d writing to 0x%x\n", ret, reg);
-out:
- mutex_unlock(&priv->mutex);
+ regmap_write(priv->regmap, reg, val);
}
static void
reg_write16(struct tda998x_priv *priv, u16 reg, u16 val)
{
- struct i2c_client *client = priv->hdmi;
- u8 buf[] = {REG2ADDR(reg), val >> 8, val};
- int ret;
-
- mutex_lock(&priv->mutex);
- ret = set_page(priv, reg);
- if (ret < 0)
- goto out;
+ u8 buf[] = {val >> 8, val};
- ret = i2c_master_send(client, buf, sizeof(buf));
- if (ret < 0)
- dev_err(&client->dev, "Error %d writing to 0x%x\n", ret, reg);
-out:
- mutex_unlock(&priv->mutex);
+ regmap_bulk_write(priv->regmap, reg, buf, ARRAY_SIZE(buf));
}
static void
reg_set(struct tda998x_priv *priv, u16 reg, u8 val)
{
- int old_val;
-
- old_val = reg_read(priv, reg);
- if (old_val >= 0)
- reg_write(priv, reg, old_val | val);
+ regmap_update_bits(priv->regmap, reg, val, val);
}
static void
reg_clear(struct tda998x_priv *priv, u16 reg, u8 val)
{
- int old_val;
-
- old_val = reg_read(priv, reg);
- if (old_val >= 0)
- reg_write(priv, reg, old_val & ~val);
+ regmap_update_bits(priv->regmap, reg, val, 0);
}
static void
@@ -816,7 +725,7 @@ static void
tda998x_write_if(struct tda998x_priv *priv, u8 bit, u16 addr,
union hdmi_infoframe *frame)
{
- u8 buf[MAX_WRITE_RANGE_BUF];
+ u8 buf[32];
ssize_t len;
len = hdmi_infoframe_pack(frame, buf, sizeof(buf));
@@ -1654,6 +1563,211 @@ static void tda998x_destroy(struct device *dev)
cec_notifier_put(priv->cec_notify);
}
+static bool tda_is_edid_data_reg(unsigned int reg)
+{
+ return (reg >= REG_EDID_DATA_0) &&
+ (reg <= REG_EDID_DATA_127);
+}
+
+static bool tda_is_precious_volatile_reg(struct device *dev, unsigned int reg)
+{
+ if (tda_is_edid_data_reg(reg))
+ return true;
+ switch (reg) {
+ case REG_INT_FLAGS_0:
+ case REG_INT_FLAGS_1:
+ case REG_INT_FLAGS_2:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool tda_is_rw_reg(unsigned int reg)
+{
+ if ((reg >= REG_IF1_HB0) && (reg <= REG_IF5_HB31))
+ return true;
+ switch (reg) {
+ case REG_MAIN_CNTRL0:
+ case REG_DDC_DISABLE:
+ case REG_CCLK_ON:
+ case REG_I2C_MASTER:
+ case REG_FEAT_POWERDOWN:
+ case REG_INT_FLAGS_0:
+ case REG_INT_FLAGS_1:
+ case REG_INT_FLAGS_2:
+ case REG_ENA_ACLK:
+ case REG_ENA_VP_0:
+ case REG_ENA_VP_1:
+ case REG_ENA_VP_2:
+ case REG_ENA_AP:
+ case REG_MUX_AP:
+ case REG_MUX_VP_VIP_OUT:
+ case REG_I2S_FORMAT:
+ case REG_PLL_SERIAL_1:
+ case REG_PLL_SERIAL_2:
+ case REG_PLL_SERIAL_3:
+ case REG_SERIALIZER:
+ case REG_BUFFER_OUT:
+ case REG_PLL_SCG1:
+ case REG_PLL_SCG2:
+ case REG_PLL_SCGN1:
+ case REG_PLL_SCGN2:
+ case REG_PLL_SCGR1:
+ case REG_PLL_SCGR2:
+ case REG_AUDIO_DIV:
+ case REG_SEL_CLK:
+ case REG_ANA_GENERAL:
+ case REG_EDID_CTRL:
+ case REG_DDC_ADDR:
+ case REG_DDC_OFFS:
+ case REG_DDC_SEGM_ADDR:
+ case REG_DDC_SEGM:
+ case REG_AIP_CNTRL_0:
+ case REG_CA_I2S:
+ case REG_LATENCY_RD:
+ case REG_ACR_CTS_0:
+ case REG_ACR_CTS_1:
+ case REG_ACR_CTS_2:
+ case REG_ACR_N_0:
+ case REG_ACR_N_1:
+ case REG_ACR_N_2:
+ case REG_CTS_N:
+ case REG_ENC_CNTRL:
+ case REG_DIP_FLAGS:
+ case REG_DIP_IF_FLAGS:
+ case REG_TX3:
+ case REG_TX4:
+ case REG_TX33:
+ case REG_CH_STAT_B(0):
+ case REG_CH_STAT_B(1):
+ case REG_CH_STAT_B(2):
+ case REG_CH_STAT_B(3):
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool tda_is_readable_reg(struct device *dev, unsigned int reg)
+{
+ if (tda_is_rw_reg(reg) || tda_is_edid_data_reg(reg))
+ return true;
+ switch (reg) {
+ case REG_VERSION_LSB:
+ case REG_VERSION_MSB:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool tda_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+ if (tda_is_rw_reg(reg))
+ return true;
+ switch (reg) {
+ case REG_CURPAGE:
+ case REG_SOFTRESET:
+ case REG_VIP_CNTRL_0:
+ case REG_VIP_CNTRL_1:
+ case REG_VIP_CNTRL_2:
+ case REG_VIP_CNTRL_3:
+ case REG_VIP_CNTRL_4:
+ case REG_VIP_CNTRL_5:
+ case REG_MAT_CONTRL:
+ case REG_VIDFORMAT:
+ case REG_REFPIX_MSB:
+ case REG_REFPIX_LSB:
+ case REG_REFLINE_MSB:
+ case REG_REFLINE_LSB:
+ case REG_NPIX_MSB:
+ case REG_NPIX_LSB:
+ case REG_NLINE_MSB:
+ case REG_NLINE_LSB:
+ case REG_VS_LINE_STRT_1_MSB:
+ case REG_VS_LINE_STRT_1_LSB:
+ case REG_VS_PIX_STRT_1_MSB:
+ case REG_VS_PIX_STRT_1_LSB:
+ case REG_VS_LINE_END_1_MSB:
+ case REG_VS_LINE_END_1_LSB:
+ case REG_VS_PIX_END_1_MSB:
+ case REG_VS_PIX_END_1_LSB:
+ case REG_VS_LINE_STRT_2_MSB:
+ case REG_VS_LINE_STRT_2_LSB:
+ case REG_VS_PIX_STRT_2_MSB:
+ case REG_VS_PIX_STRT_2_LSB:
+ case REG_VS_LINE_END_2_MSB:
+ case REG_VS_LINE_END_2_LSB:
+ case REG_VS_PIX_END_2_MSB:
+ case REG_VS_PIX_END_2_LSB:
+ case REG_HS_PIX_START_MSB:
+ case REG_HS_PIX_START_LSB:
+ case REG_HS_PIX_STOP_MSB:
+ case REG_HS_PIX_STOP_LSB:
+ case REG_VWIN_START_1_MSB:
+ case REG_VWIN_START_1_LSB:
+ case REG_VWIN_END_1_MSB:
+ case REG_VWIN_END_1_LSB:
+ case REG_VWIN_START_2_MSB:
+ case REG_VWIN_START_2_LSB:
+ case REG_VWIN_END_2_MSB:
+ case REG_VWIN_END_2_LSB:
+ case REG_DE_START_MSB:
+ case REG_DE_START_LSB:
+ case REG_DE_STOP_MSB:
+ case REG_DE_STOP_LSB:
+ case REG_TBG_CNTRL_0:
+ case REG_TBG_CNTRL_1:
+ case REG_ENABLE_SPACE:
+ case REG_HVF_CNTRL_0:
+ case REG_HVF_CNTRL_1:
+ case REG_RPT_CNTRL:
+ case REG_AIP_CLKSEL:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static const struct reg_default tda_reg_default[] = {
+ { REG_CURPAGE, 0xff },
+};
+
+static const struct regmap_range_cfg hdmi_range_cfg[] = {
+ {
+ .range_min = 0x00,
+ .range_max = REG_TX33,
+ .selector_reg = REG_CURPAGE,
+ .selector_mask = 0xff,
+ .selector_shift = 0,
+ .window_start = 0,
+ .window_len = 0x100,
+ },
+};
+
+/* Paged register scheme, with a write-only page register (CURPAGE).
+ * Make this work by marking CURPAGE write-only and cacheable. Give it
+ * an invalid page default value, which guarantees that it will get written to
+ * the first time we read/write the registers.
+ */
+
+static const struct regmap_config hdmi_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .ranges = hdmi_range_cfg,
+ .num_ranges = ARRAY_SIZE(hdmi_range_cfg),
+ .max_register = REG_TX33,
+
+ .cache_type = REGCACHE_RBTREE,
+ .volatile_reg = tda_is_precious_volatile_reg,
+ .precious_reg = tda_is_precious_volatile_reg,
+ .readable_reg = tda_is_readable_reg,
+ .writeable_reg = tda_is_writeable_reg,
+ .reg_defaults = tda_reg_default,
+ .num_reg_defaults = ARRAY_SIZE(tda_reg_default),
+};
+
static int tda998x_create(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
@@ -1666,10 +1780,15 @@ static int tda998x_create(struct device *dev)
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
+ priv->regmap = devm_regmap_init_i2c(client, &hdmi_regmap_config);
+ if (IS_ERR(priv->regmap)) {
+ ret = PTR_ERR(priv->regmap);
+ dev_err(dev, "Failed to allocate register map: %d\n", ret);
+ return ret;
+ }
dev_set_drvdata(dev, priv);
- mutex_init(&priv->mutex); /* protect the page access */
mutex_init(&priv->audio_mutex); /* protect access from audio thread */
mutex_init(&priv->edid_mutex);
INIT_LIST_HEAD(&priv->bridge.list);
@@ -1683,7 +1802,6 @@ static int tda998x_create(struct device *dev)
/* CEC I2C address bound to TDA998x I2C addr by configuration pins */
priv->cec_addr = 0x34 + (client->addr & 0x03);
- priv->current_page = 0xff;
priv->hdmi = client;
/* wake up the device: */
--
2.17.1
More information about the dri-devel
mailing list