Re: [PATCH v1 2/2] drm/xe/mcu_i2c: Take over control of the controller enabling

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Wed, Jun 24, 2026 at 01:14:02PM +0200, Raag Jadav wrote:
> On Wed, Jun 24, 2026 at 01:13:56PM +0300, Heikki Krogerus wrote:
> > On Tue, Jun 23, 2026 at 04:39:05PM +0200, Raag Jadav wrote:
> > > On Mon, Jun 22, 2026 at 01:47:59PM +0200, Heikki Krogerus wrote:
> > > > Some platforms make an assumption that the i2c controller's
> > > > enabled state indicates also the power state of the
> > > > controller. This can create a problem when the controller is
> > > > in disabled state, because the hardware may assume
> > > > incorrectly that it is then also in low-power state.
> > > > 
> > > > To fix this, the controller is kept enabled by taking over
> > > > the IC_ENABLE register. The controller has to be disabled
> > > > when the configuration is updated and when the target
> > > > address or the slave address are assigned, so disabling it
> > > > when IC_CON, IC_TAR or IC_SAR registers are programmed, and
> > > > then re-enabling it again.
> > > 
> > > ...
> > > 
> > > >  static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
> > > >  {
> > > >  	struct xe_i2c *i2c = context;
> > > >  
> > > > -	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
> > > > +	switch (reg) {
> > > 
> > > Curious, should I expect DW_IC_INTR_MASK case here which skips the MMIO?
> > 
> > Probable not. We have the ACCESS_POLLING flag set, so the
> > i2c-designware will only write 0 to that register. Check
> > __i2c_dw_write_intr_mask().
> 
> Yes, but I'm not sure if all the writes to DW_IC_INTR_MASK have been
> converted to use that helper yet, or did I miss something?

It is only written directly in slave mode, but since slave mode is not
supported with ACCESS_POLLING at the moment, we will never reach that
code.

I'm hoping we will never have to support ACCESS_POLLING in slave mode,
but I'll send a patch (not as part of this series) in any case where
I'll make sure DW_IC_INTR_MASK is only written from that helper.

Thanks,

-- 
heikki