On 15.06.2023 00:24, Andi Shyti wrote:
> Hi Heiner,
>
> On Sat, Mar 04, 2023 at 10:31:23PM +0100, Heiner Kallweit wrote:
>> When entering the shutdown/remove/suspend callbacks, at first we should
>> ensure that transfers are finished and I2C core can't start further
>> transfers. Use i2c_mark_adapter_suspended() for this purpose.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>> ---
>> drivers/i2c/busses/i2c-i801.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
>> index ac5326747..d6a0c3b53 100644
>> --- a/drivers/i2c/busses/i2c-i801.c
>> +++ b/drivers/i2c/busses/i2c-i801.c
>> @@ -1773,6 +1773,8 @@ static void i801_remove(struct pci_dev *dev)
>> {
>> struct i801_priv *priv = pci_get_drvdata(dev);
>>
>> + i2c_mark_adapter_suspended(&priv->adapter);
>> +
>> outb_p(priv->original_hstcnt, SMBHSTCNT(priv));
>> i801_disable_host_notify(priv);
>> i801_del_mux(priv);
>> @@ -1796,6 +1798,8 @@ static void i801_shutdown(struct pci_dev *dev)
>> {
>> struct i801_priv *priv = pci_get_drvdata(dev);
>>
>> + i2c_mark_adapter_suspended(&priv->adapter);
>> +
>
> is this really needed in the shutdown and remove function?
>
I think yes. Otherwise we may interrupt an active transfer, or a user
may start a transfer whilst we are in cleanup.
Note: i2c_mark_adapter_suspended() takes the I2C bus lock, therefore it
will sleep until an active transfer is finished.
> I'm OK with it, though.
>
>> /* Restore config registers to avoid hard hang on some systems */
>> outb_p(priv->original_hstcnt, SMBHSTCNT(priv));
>> i801_disable_host_notify(priv);
>> @@ -1807,6 +1811,7 @@ static int i801_suspend(struct device *dev)
>> {
>> struct i801_priv *priv = dev_get_drvdata(dev);
>>
>> + i2c_mark_adapter_suspended(&priv->adapter);
>> outb_p(priv->original_hstcnt, SMBHSTCNT(priv));
>> pci_write_config_byte(priv->pci_dev, SMBHSTCFG, priv->original_hstcfg);
>> return 0;
>> @@ -1818,6 +1823,7 @@ static int i801_resume(struct device *dev)
>>
>> i801_setup_hstcfg(priv);
>> i801_enable_host_notify(&priv->adapter);
>> + i2c_mark_adapter_resumed(&priv->adapter);
>
> BTW, I see that very few drivers are using suspended and resumed
> and I wonder why. Should these perhaps be added in the basic pm
> functions?
>
For my understanding, which functions are you referring to?
> I'm OK to r-b this, but i want first Jean to give it an ack.
>
> Andi
