[RFC] drm/bridge/sii902x: Fix EDID readback
Fabrizio Castro
fabrizio.castro at bp.renesas.com
Wed Oct 31 16:55:53 UTC 2018
Hello Linus,
> Subject: Re: [RFC] drm/bridge/sii902x: Fix EDID readback
>
> Hi Fabrizio,
>
> thanks for your patch!
Thank you for your feedback!
>
> On Wed, Oct 31, 2018 at 1:58 PM Fabrizio Castro
> <fabrizio.castro at bp.renesas.com> wrote:
>
> > While adding SiI9022A support to the iwg23s board it came up
> > that when the HDMI transmitter is in pass through mode the
> > device is not compliant with the I2C specification anymore,
> > as it requires a far bigger tbuf due to a delay the HDMI
> > transmitter is adding when relaying the STOP condition on the
> > monitor i2c side of things. When not providing an appropriate
> > delay after the STOP condition the i2c bus would get stuck.
> > Also, any other traffic on the bus while talking to the monitor
> > may cause the transaction to fail or even cause issues with the
> > i2c bus as well.
> >
> > I2c-gates seemed to reach consent as a possible way to address
> > these issues, and as such this patch is implementing a solution
> > based on that. Since others are clearly relying on the current
> > implementation of the driver, this patch won't require any DT
> > changes.
> >
> > Since we don't want any interference during the DDC Bus
> > Request/Grant procedure and while talking to the monitor, we have
> > to use the adapter locking primitives rather than the i2c-mux
> > locking primitives, but in order to achieve that I had to get
> > rid of regmap.
>
> Why did you have to remove regmap? It is perfectly possible
> to override any reading/writing operations locally if you don't
> want to use the regmap i2c variants.
>
> The code in your patch does not make it evident to me exactly
> where the problem is with regmap, also I bet the regmap
> maintainer would love to hear about it so we can attempt to
> fix it there instead of locally in this driver.
>
> AFAICT there is some locked vs unlocked business and I
> strongly hope there is some way to simply pass that down
> into whatever functions regmap end up calling so we don't
> have to change all call sites.
Yeah, there is some issue with locking here, and that's coming down
from the fact that when we call into drm_get_edid, we implicitly call
i2c_transfer which ultimately locks the i2c adapter, and then calls
into our sii902x_i2c_bypass_select, which in turn calls into the regmap
functions and therefore we try and lock the same i2c adapter again,
driving the system into a deadlock.
Having the option of using "unlocked" flavours of reads and writes
is what we need here, but looking at drivers/base/regmap/regmap-i2c.c
I couldn't find anything suitable for my case, maybe Mark could advise
on this one? I am sure I overlooked something here, is there a better
way to address this?
>
> (So please include Mark Brown on CC in future iterations.)
>
> > Signed-off-by: Fabrizio Castro <fabrizio.castro at bp.renesas.com>
> > ---
> > Dear All,
> >
> > this is a follow up to:
> > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg32072.html
> >
> > Comments very welcome!
>
> At the very least I think the patch needs to be split in two,
> one dealing with the locking business and one dealing with
> the buffer size. As it looks now it is very hard to review.
The change with the buffer size comes down from sii902x_bulk_write implementation,
which btw is the only function that doesn't resembles its regmap equivalent, in terms
of parameters.
>
> >
> > Thanks,
> > Fab
> >
> > drivers/gpu/drm/bridge/sii902x.c | 404 ++++++++++++++++++++++++++-------------
> > 1 file changed, 274 insertions(+), 130 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > index e59a135..137a05a 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -21,9 +21,9 @@
> > */
> >
> > #include <linux/gpio/consumer.h>
> > +#include <linux/i2c-mux.h>
> > #include <linux/i2c.h>
> > #include <linux/module.h>
> > -#include <linux/regmap.h>
> >
> > #include <drm/drmP.h>
> > #include <drm/drm_atomic_helper.h>
> > @@ -82,12 +82,130 @@
> >
> > struct sii902x {
> > struct i2c_client *i2c;
> > - struct regmap *regmap;
> > struct drm_bridge bridge;
> > struct drm_connector connector;
> > struct gpio_desc *reset_gpio;
> > + struct i2c_mux_core *i2cmux;
> > };
> >
> > +static int sii902x_transfer_read(struct i2c_client *i2c, u8 reg, u8 *val,
> > + u16 len, bool locked)
> > +{
> > + struct i2c_msg xfer[] = {
> > + {
> > + .addr = i2c->addr,
> > + .flags = 0,
> > + .len = 1,
> > + .buf = ®,
> > + }, {
> > + .addr = i2c->addr,
> > + .flags = I2C_M_RD,
> > + .len = len,
> > + .buf = val,
> > + }
> > + };
> > + unsigned char xfers = ARRAY_SIZE(xfer);
> > + int ret, retries = 5;
> > +
> > + do {
> > + if (locked)
> > + ret = i2c_transfer(i2c->adapter, xfer, xfers);
> > + else
> > + ret = __i2c_transfer(i2c->adapter, xfer, xfers);
> > + if (ret < 0)
> > + return ret;
> > + } while (ret != xfers && --retries);
> > + return ret == xfers ? 0 : -1;
> > +}
> > +
> > +static int sii902x_bulk_read(struct i2c_client *i2c, u8 reg, u8 *val, u16 len)
> > +{
> > + return sii902x_transfer_read(i2c, reg, val, len, true);
> > +}
> > +
> > +static int __sii902x_bulk_read(struct i2c_client *i2c, u8 reg, u8 *val, u16 len)
> > +{
> > + return sii902x_transfer_read(i2c, reg, val, len, false);
> > +}
>
> I'm not a fan of __functions because it is syntactically
> ambigous what that means.
>
> Example set_bit() vs __set_bit() - is it obvious from context
> that the latter is non-atomic? No.
>
> Give these functions names that indicate exactly what they
> do and why, please.
>
> *_locked and *_unlocked variants?
I agree, will do that
>
> > +static int sii902x_read(struct i2c_client *i2c, u8 reg, u8 *val)
> > +{
> > + return sii902x_bulk_read(i2c, reg, val, 1);
> > +}
> > +
> > +static int __sii902x_read(struct i2c_client *i2c, u8 reg, u8 *val)
> > +{
> > + return __sii902x_bulk_read(i2c, reg, val, 1);
> > +}
>
> Dito
>
> > +static int sii902x_transfer_write(struct i2c_client *i2c, u8 *val,
> > + u16 len, bool locked)
> > +{
> > + struct i2c_msg xfer = {
> > + .addr = i2c->addr,
> > + .flags = 0,
> > + .len = len,
> > + .buf = val,
> > + };
> > + int ret, retries = 5;
> > +
> > + do {
> > + if (locked)
> > + ret = i2c_transfer(i2c->adapter, &xfer, 1);
> > + else
> > + ret = __i2c_transfer(i2c->adapter, &xfer, 1);
>
> This locked variable seems to be the magic dust so
> please document it thorughly.
Will do
>
> > + if (ret < 0)
> > + return ret;
> > + } while (!ret && --retries);
> > + return !ret ? -1 : 0;
> > +}
> > +
> > +static int sii902x_bulk_write(struct i2c_client *i2c, u8 *val, u16 len)
> > +{
> > + return sii902x_transfer_write(i2c, val, len, true);
> > +}
> > +
> > +static int sii902x_write(struct i2c_client *i2c, u8 reg, u8 val)
> > +{
> > + u8 data[2] = {reg, val};
> > +
> > + return sii902x_transfer_write(i2c, data, 2, true);
> > +}
> > +
> > +static int __sii902x_write(struct i2c_client *i2c, u8 reg, u8 val)
> > +{
> > + u8 data[2] = {reg, val};
> > +
> > + return sii902x_transfer_write(i2c, data, 2, false);
> > +}
> > +
> > +static int sii902x_update_bits(struct i2c_client *i2c, u8 reg, u8 mask, u8 val)
> > +{
> > + int ret;
> > + u8 status;
> > +
> > + ret = sii902x_read(i2c, reg, &status);
> > + if (ret)
> > + return ret;
> > + status &= ~mask;
> > + status |= val & mask;
> > + return sii902x_write(i2c, reg, status);
> > +}
> > +
> > +static int __sii902x_update_bits(struct i2c_client *i2c, u8 reg, u8 mask,
> > + u8 val)
> > +{
> > + int ret;
> > + u8 status;
> > +
> > + ret = __sii902x_read(i2c, reg, &status);
> > + if (ret)
> > + return ret;
> > + status &= ~mask;
> > + status |= val & mask;
> > + return __sii902x_write(i2c, reg, status);
> > +}
>
> Dito.
>
> So now all read, write and rmw functions are duplicated.
>
> > static inline struct sii902x *bridge_to_sii902x(struct drm_bridge *bridge)
> > {
> > return container_of(bridge, struct sii902x, bridge);
> > @@ -115,9 +233,9 @@ static enum drm_connector_status
> > sii902x_connector_detect(struct drm_connector *connector, bool force)
> > {
> > struct sii902x *sii902x = connector_to_sii902x(connector);
> > - unsigned int status;
> > + u8 status;
> >
> > - regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
> > + sii902x_read(sii902x->i2c, SII902X_INT_STATUS, &status);
> >
> > return (status & SII902X_PLUGGED_STATUS) ?
> > connector_status_connected : connector_status_disconnected;
> > @@ -135,41 +253,11 @@ static const struct drm_connector_funcs sii902x_connector_funcs = {
> > static int sii902x_get_modes(struct drm_connector *connector)
> > {
> > struct sii902x *sii902x = connector_to_sii902x(connector);
> > - struct regmap *regmap = sii902x->regmap;
> > u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > - struct device *dev = &sii902x->i2c->dev;
> > - unsigned long timeout;
> > - unsigned int retries;
> > - unsigned int status;
> > struct edid *edid;
> > - int num = 0;
> > - int ret;
> > -
> > - ret = regmap_update_bits(regmap, SII902X_SYS_CTRL_DATA,
> > - SII902X_SYS_CTRL_DDC_BUS_REQ,
> > - SII902X_SYS_CTRL_DDC_BUS_REQ);
> > - if (ret)
> > - return ret;
> > + int num = 0, ret;
> >
> > - timeout = jiffies +
> > - msecs_to_jiffies(SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS);
> > - do {
> > - ret = regmap_read(regmap, SII902X_SYS_CTRL_DATA, &status);
> > - if (ret)
> > - return ret;
> > - } while (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD) &&
> > - time_before(jiffies, timeout));
> > -
> > - if (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> > - dev_err(dev, "failed to acquire the i2c bus\n");
> > - return -ETIMEDOUT;
> > - }
> > -
> > - ret = regmap_write(regmap, SII902X_SYS_CTRL_DATA, status);
> > - if (ret)
> > - return ret;
> > -
> > - edid = drm_get_edid(connector, sii902x->i2c->adapter);
> > + edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
> > drm_connector_update_edid_property(connector, edid);
> > if (edid) {
> > num = drm_add_edid_modes(connector, edid);
> > @@ -181,42 +269,6 @@ static int sii902x_get_modes(struct drm_connector *connector)
> > if (ret)
> > return ret;
> >
> > - /*
> > - * Sometimes the I2C bus can stall after failure to use the
> > - * EDID channel. Retry a few times to see if things clear
> > - * up, else continue anyway.
> > - */
> > - retries = 5;
> > - do {
> > - ret = regmap_read(regmap, SII902X_SYS_CTRL_DATA,
> > - &status);
> > - retries--;
> > - } while (ret && retries);
> > - if (ret)
> > - dev_err(dev, "failed to read status (%d)\n", ret);
> > -
> > - ret = regmap_update_bits(regmap, SII902X_SYS_CTRL_DATA,
> > - SII902X_SYS_CTRL_DDC_BUS_REQ |
> > - SII902X_SYS_CTRL_DDC_BUS_GRTD, 0);
> > - if (ret)
> > - return ret;
> > -
> > - timeout = jiffies +
> > - msecs_to_jiffies(SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS);
> > - do {
> > - ret = regmap_read(regmap, SII902X_SYS_CTRL_DATA, &status);
> > - if (ret)
> > - return ret;
> > - } while (status & (SII902X_SYS_CTRL_DDC_BUS_REQ |
> > - SII902X_SYS_CTRL_DDC_BUS_GRTD) &&
> > - time_before(jiffies, timeout));
> > -
> > - if (status & (SII902X_SYS_CTRL_DDC_BUS_REQ |
> > - SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> > - dev_err(dev, "failed to release the i2c bus\n");
> > - return -ETIMEDOUT;
> > - }
> > -
> > return num;
> > }
> >
> > @@ -237,20 +289,20 @@ static void sii902x_bridge_disable(struct drm_bridge *bridge)
> > {
> > struct sii902x *sii902x = bridge_to_sii902x(bridge);
> >
> > - regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> > - SII902X_SYS_CTRL_PWR_DWN,
> > - SII902X_SYS_CTRL_PWR_DWN);
> > + sii902x_update_bits(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + SII902X_SYS_CTRL_PWR_DWN,
> > + SII902X_SYS_CTRL_PWR_DWN);
> > }
> >
> > static void sii902x_bridge_enable(struct drm_bridge *bridge)
> > {
> > struct sii902x *sii902x = bridge_to_sii902x(bridge);
> >
> > - regmap_update_bits(sii902x->regmap, SII902X_PWR_STATE_CTRL,
> > - SII902X_AVI_POWER_STATE_MSK,
> > - SII902X_AVI_POWER_STATE_D(0));
> > - regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> > - SII902X_SYS_CTRL_PWR_DWN, 0);
> > + sii902x_update_bits(sii902x->i2c, SII902X_PWR_STATE_CTRL,
> > + SII902X_AVI_POWER_STATE_MSK,
> > + SII902X_AVI_POWER_STATE_D(0));
> > + sii902x_update_bits(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + SII902X_SYS_CTRL_PWR_DWN, 0);
> > }
> >
> > static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> > @@ -258,25 +310,25 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> > struct drm_display_mode *adj)
> > {
> > struct sii902x *sii902x = bridge_to_sii902x(bridge);
> > - struct regmap *regmap = sii902x->regmap;
> > - u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
> > + u8 buf[HDMI_INFOFRAME_SIZE(AVI) + 1];
>
> Maybe this stuff should be a separate change?
This change is due to the fact that sii902x_bulk_write takes an array of u8, with the first element
being the register address, this makes sii902x_bulk_write not exactly the same as regmap_bulk_write.
Mark may point out something better regmap related, so maybe there is no even need for this.
Again, thank you very much for your comments, I am not going to send a v2 out straight away, I would
like to hear at least from Wolfram and Peter about the way I have implement the i2c-gate as it is a bit
unconventional, and then it would be great to have Mark's opinion on the locking bit.
Fab
>
> > struct hdmi_avi_infoframe frame;
> > int ret;
> >
> > - buf[0] = adj->clock;
> > - buf[1] = adj->clock >> 8;
> > - buf[2] = adj->vrefresh;
> > - buf[3] = 0x00;
> > - buf[4] = adj->hdisplay;
> > - buf[5] = adj->hdisplay >> 8;
> > - buf[6] = adj->vdisplay;
> > - buf[7] = adj->vdisplay >> 8;
> > - buf[8] = SII902X_TPI_CLK_RATIO_1X | SII902X_TPI_AVI_PIXEL_REP_NONE |
> > + buf[0] = SII902X_TPI_VIDEO_DATA;
> > + buf[1] = adj->clock;
> > + buf[2] = adj->clock >> 8;
> > + buf[3] = adj->vrefresh;
> > + buf[4] = 0x00;
> > + buf[5] = adj->hdisplay;
> > + buf[6] = adj->hdisplay >> 8;
> > + buf[7] = adj->vdisplay;
> > + buf[8] = adj->vdisplay >> 8;
> > + buf[9] = SII902X_TPI_CLK_RATIO_1X | SII902X_TPI_AVI_PIXEL_REP_NONE |
> > SII902X_TPI_AVI_PIXEL_REP_BUS_24BIT;
> > - buf[9] = SII902X_TPI_AVI_INPUT_RANGE_AUTO |
> > - SII902X_TPI_AVI_INPUT_COLORSPACE_RGB;
> > + buf[10] = SII902X_TPI_AVI_INPUT_RANGE_AUTO |
> > + SII902X_TPI_AVI_INPUT_COLORSPACE_RGB;
> >
> > - ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
> > + ret = sii902x_bulk_write(sii902x->i2c, buf, 11);
> > if (ret)
> > return;
> >
> > @@ -286,16 +338,17 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> > return;
> > }
> >
> > - ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
> > + ret = hdmi_avi_infoframe_pack(&frame, buf + 1, sizeof(buf) - 1);
> > if (ret < 0) {
> > DRM_ERROR("failed to pack AVI infoframe: %d\n", ret);
> > return;
> > }
> >
> > + buf[0] = SII902X_TPI_AVI_INFOFRAME;
> > /* Do not send the infoframe header, but keep the CRC field. */
> > - regmap_bulk_write(regmap, SII902X_TPI_AVI_INFOFRAME,
> > - buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
> > - HDMI_AVI_INFOFRAME_SIZE + 1);
> > + sii902x_bulk_write(sii902x->i2c,
> > + buf + HDMI_INFOFRAME_HEADER_SIZE,
> > + HDMI_AVI_INFOFRAME_SIZE + 1);
> > }
> >
> > static int sii902x_bridge_attach(struct drm_bridge *bridge)
> > @@ -336,29 +389,13 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = {
> > .enable = sii902x_bridge_enable,
> > };
> >
> > -static const struct regmap_range sii902x_volatile_ranges[] = {
> > - { .range_min = 0, .range_max = 0xff },
> > -};
> > -
> > -static const struct regmap_access_table sii902x_volatile_table = {
> > - .yes_ranges = sii902x_volatile_ranges,
> > - .n_yes_ranges = ARRAY_SIZE(sii902x_volatile_ranges),
> > -};
> > -
> > -static const struct regmap_config sii902x_regmap_config = {
> > - .reg_bits = 8,
> > - .val_bits = 8,
> > - .volatile_table = &sii902x_volatile_table,
> > - .cache_type = REGCACHE_NONE,
> > -};
> > -
> > static irqreturn_t sii902x_interrupt(int irq, void *data)
> > {
> > struct sii902x *sii902x = data;
> > - unsigned int status = 0;
> > + u8 status = 0;
> >
> > - regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
> > - regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
> > + sii902x_read(sii902x->i2c, SII902X_INT_STATUS, &status);
> > + sii902x_write(sii902x->i2c, SII902X_INT_STATUS, status);
> >
> > if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev)
> > drm_helper_hpd_irq_event(sii902x->bridge.dev);
> > @@ -366,23 +403,111 @@ static irqreturn_t sii902x_interrupt(int irq, void *data)
> > return IRQ_HANDLED;
> > }
> >
> > +static int sii902x_i2c_bypass_select(struct i2c_mux_core *mux, u32 chan_id)
> > +{
> > + struct sii902x *sii902x = i2c_mux_priv(mux);
> > + struct device *dev = &sii902x->i2c->dev;
> > + unsigned long timeout;
> > + u8 status;
> > + int ret;
> > +
> > + ret = __sii902x_update_bits(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + SII902X_SYS_CTRL_DDC_BUS_REQ,
> > + SII902X_SYS_CTRL_DDC_BUS_REQ);
> > +
> > + timeout = jiffies +
> > + msecs_to_jiffies(SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS);
> > + do {
> > + ret = __sii902x_read(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + &status);
> > + if (ret)
> > + return ret;
> > + } while (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD) &&
> > + time_before(jiffies, timeout));
> > +
> > + if (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> > + dev_err(dev, "Failed to acquire the i2c bus\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + ret = __sii902x_write(sii902x->i2c, SII902X_SYS_CTRL_DATA, status);
> > + if (ret)
> > + return ret;
> > + return 0;
> > +}
> > +
> > +static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id)
> > +{
> > + struct sii902x *sii902x = i2c_mux_priv(mux);
> > + struct device *dev = &sii902x->i2c->dev;
> > + unsigned long timeout;
> > + unsigned int retries;
> > + u8 status;
> > + int ret;
> > +
> > + udelay(30);
> > +
> > + /*
> > + * Sometimes the I2C bus can stall after failure to use the
> > + * EDID channel. Retry a few times to see if things clear
> > + * up, else continue anyway.
> > + */
> > + retries = 5;
> > + do {
> > + ret = __sii902x_read(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + &status);
> > + retries--;
> > + } while (ret && retries);
> > + if (ret) {
> > + dev_err(dev, "failed to read status (%d)\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = __sii902x_update_bits(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + SII902X_SYS_CTRL_DDC_BUS_REQ |
> > + SII902X_SYS_CTRL_DDC_BUS_GRTD, 0);
> > + if (ret)
> > + return ret;
> > +
> > + timeout = jiffies +
> > + msecs_to_jiffies(SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS);
> > + do {
> > + ret = __sii902x_read(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + &status);
> > + if (ret)
> > + return ret;
> > + } while (status & (SII902X_SYS_CTRL_DDC_BUS_REQ |
> > + SII902X_SYS_CTRL_DDC_BUS_GRTD) &&
> > + time_before(jiffies, timeout));
> > +
> > + if (status & (SII902X_SYS_CTRL_DDC_BUS_REQ |
> > + SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> > + dev_err(dev, "failed to release the i2c bus\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int sii902x_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > struct device *dev = &client->dev;
> > - unsigned int status = 0;
> > struct sii902x *sii902x;
> > - u8 chipid[4];
> > + u8 chipid[4], status = 0;
> > int ret;
> >
> > + ret = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
> > + if (!ret) {
> > + dev_err(dev, "I2C adapter not suitable\n");
> > + return -EIO;
> > + }
> > +
> > sii902x = devm_kzalloc(dev, sizeof(*sii902x), GFP_KERNEL);
> > if (!sii902x)
> > return -ENOMEM;
> >
> > sii902x->i2c = client;
> > - sii902x->regmap = devm_regmap_init_i2c(client, &sii902x_regmap_config);
> > - if (IS_ERR(sii902x->regmap))
> > - return PTR_ERR(sii902x->regmap);
> >
> > sii902x->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > GPIOD_OUT_LOW);
> > @@ -394,14 +519,14 @@ static int sii902x_probe(struct i2c_client *client,
> >
> > sii902x_reset(sii902x);
> >
> > - ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> > + ret = sii902x_write(sii902x->i2c, SII902X_REG_TPI_RQB, 0x0);
> > if (ret)
> > return ret;
> >
> > - ret = regmap_bulk_read(sii902x->regmap, SII902X_REG_CHIPID(0),
> > - &chipid, 4);
> > + ret = sii902x_bulk_read(sii902x->i2c, SII902X_REG_CHIPID(0),
> > + chipid, 4);
> > if (ret) {
> > - dev_err(dev, "regmap_read failed %d\n", ret);
> > + dev_err(dev, "Can't read chipid (error = %d)\n", ret);
> > return ret;
> > }
> >
> > @@ -412,12 +537,12 @@ static int sii902x_probe(struct i2c_client *client,
> > }
> >
> > /* Clear all pending interrupts */
> > - regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
> > - regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
> > + sii902x_read(sii902x->i2c, SII902X_INT_STATUS, &status);
> > + sii902x_write(sii902x->i2c, SII902X_INT_STATUS, status);
> >
> > if (client->irq > 0) {
> > - regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
> > - SII902X_HOTPLUG_EVENT);
> > + sii902x_write(sii902x->i2c, SII902X_INT_ENABLE,
> > + SII902X_HOTPLUG_EVENT);
> >
> > ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > sii902x_interrupt,
> > @@ -433,6 +558,22 @@ static int sii902x_probe(struct i2c_client *client,
> >
> > i2c_set_clientdata(client, sii902x);
> >
> > + sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> > + 1, 0, I2C_MUX_GATE,
> > + sii902x_i2c_bypass_select,
> > + sii902x_i2c_bypass_deselect);
> > + if (!sii902x->i2cmux) {
> > + dev_err(dev, "failed to allocate I2C mux\n");
> > + return -ENOMEM;
> > + }
> > +
> > + sii902x->i2cmux->priv = sii902x;
> > + ret = i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0);
> > + if (ret) {
> > + dev_err(dev, "Couldn't add i2c mux adapter\n");
> > + return ret;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -441,6 +582,9 @@ static int sii902x_remove(struct i2c_client *client)
> > {
> > struct sii902x *sii902x = i2c_get_clientdata(client);
> >
> > + if (sii902x->i2cmux)
> > + i2c_mux_del_adapters(sii902x->i2cmux);
> > +
> > drm_bridge_remove(&sii902x->bridge);
> >
> > return 0;
>
> Yours,
> Linus Walleij
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
More information about the dri-devel
mailing list