On Tue, Jun 23, 2026 at 01:54:21PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 22, 2026 at 01:47:58PM +0200, Heikki Krogerus wrote:
> > Some devices that are attached to the I2C controller use the
> > SMBus Alert signal for example to inform the host about
> > thermal events, so registering the default SMBus Alert
> > device device for them. The alert device makes sure that
> > the alert is processed and passed to the correct I2C client
> > driver.
>
> ...
>
> > + if (i2c->smbus_alert)
>
> Unneeded. Also the below incorporates error pointer cases.
>
> > + i2c_unregister_device(i2c->smbus_alert);
> > +
> > if (!i2c->irqdomain)
> > return;
>
> ...
>
> > +++ b/drivers/gpu/drm/xe/xe_i2c_smbus.c
> > @@ -0,0 +1,59 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Intel Xe SMBus handling
> > + *
> > + * Copyright (C) 2026 Intel Corporation.
> > + */
>
> + bits.h // BIT()
> + err.h // IS_ERR()
>
> > +#include <linux/i2c-smbus.h>
> > +#include <linux/regmap.h>
> > +#include <linux/types.h>
>
> ...
>
> > +static struct i2c_smbus_alert_setup xe_i2c_smbus_setup;
> > +
> > +int xe_i2c_register_smbus_alert(struct xe_i2c *i2c)
> > +{
> > + struct i2c_client *alert;
> > +
> > + alert = i2c_new_smbus_alert_device(i2c->adapter, &xe_i2c_smbus_setup);
> > + if (IS_ERR(alert))
> > + return PTR_ERR(alert);
> > +
> > + i2c->smbus_alert = alert;
> > +
> > + return 0;
> > +}
>
> As per above comment, this IS_ERR() is not need, as the removal API is
> optional-aware.
> OTOH, the other NULL-check in the driver in that case needs to be
> IS_ERR_OR_NULL().
>
> But I'm not sure if you want to have alert setup to always succeed.
Thanks Andy. I'll get these fixed.
I have to refactor the code a little. It does not seem to be possible
to try to conditionally include this file based on CONFIG_I2C_SMBUS
like I'm doing now, so I'm going to propose that CONFIG_XE selects
CONFIG_I2C_SMBUS in v2. Since there are only two functions here, I'll
just move them to xe_i2c.c
cheers,
--
heikki
