On Jul 07 2015 or thereabouts, Jean Delvare wrote: > On Tue, 7 Jul 2015 10:23:33 -0400, Benjamin Tissoires wrote: > > Hi Jean, > > > > On Jun 29 2015 or thereabouts, Jean Delvare wrote: > > > Hi Benjamin, > > > > > > On Tue, 23 Jun 2015 14:58:18 -0400, Benjamin Tissoires wrote: > > > > SMBus Host Notify allows a slave device to act as a master on a bus to > > > > notify the host of an interrupt. The functionality is directly > > > > implemented > > > > in the firmware so we just export a toggle function and rely on the > > > > adapter to actually support this. > > > > > > Why does it need to be toggled? I mean, if the controller supports it, > > > why don't we just enable the feature all the time? > > > > That is definitively a solution. I however had once a problem with > > suspend/resume where i2c_i801 was not able to reset the Host Notify > > feature, but that was maybe a side effect of unloading/reloading the > > module tons of times during the session. > > > > I will work on that and submit a v2 this week. > > > > I still wonder if you think we should expose a KConfig parameter for > > this or not. I would prefer not to (because it is handled in hardware > > and people might screw up their input drivers), but I'd prefer have your > > opinion on this first. > > What would the Kconfig option do exactly? As a general rule, the fewer > Kconfig options the better. We already have I2C_SMBUS for the SMBus > protocol specifics and Host Notify is one of these.
Yeah, that was a dumb suggestion. i2c_i801 is a SMBus driver, so we should be able to enable Host Notify unconditionally. > > > > > Signed-off-by: Benjamin Tissoires <[email protected]> > > > > --- > > > > Documentation/i2c/smbus-protocol | 4 ++++ > > > > drivers/i2c/i2c-core.c | 26 ++++++++++++++++++++++++++ > > > > include/linux/i2c.h | 8 ++++++++ > > > > include/uapi/linux/i2c.h | 1 + > > > > 4 files changed, 39 insertions(+) > > > > > > > > diff --git a/Documentation/i2c/smbus-protocol > > > > b/Documentation/i2c/smbus-protocol > > > > index 6012b12..af4e4b9 100644 > > > > --- a/Documentation/i2c/smbus-protocol > > > > +++ b/Documentation/i2c/smbus-protocol > > > > @@ -199,6 +199,10 @@ alerting device's address. > > > > > > > > [S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P] > > > > > > > > +I2C drivers for devices which can trigger SMBus Host Notify should call > > > > +i2c_smbus_toggle_host_notify() to enable SMBUS Host Notify on the > > > > adapter > > > > +and implement the optional alert() callback. > > > > + > > > > > > > > Packet Error Checking (PEC) > > > > =========================== > > > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c > > > > index 987c124..eaaf5b0 100644 > > > > --- a/drivers/i2c/i2c-core.c > > > > +++ b/drivers/i2c/i2c-core.c > > > > @@ -2959,6 +2959,32 @@ int i2c_slave_unregister(struct i2c_client > > > > *client) > > > > EXPORT_SYMBOL_GPL(i2c_slave_unregister); > > > > #endif > > > > > > > > +int i2c_smbus_toggle_host_notify(struct i2c_client *client, bool state) > > > > +{ > > > > + int ret; > > > > + > > > > + if (!client) > > > > + return -EINVAL; > > > > + > > > > + if (!(client->flags & I2C_CLIENT_TEN)) { > > > > + /* Enforce stricter address checking */ > > > > + ret = i2c_check_addr_validity(client->addr); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > + > > > > + if (!client->adapter->algo->toggle_smbus_host_notify) > > > > + return -EOPNOTSUPP; > > > > + > > > > + i2c_lock_adapter(client->adapter); > > > > + ret = > > > > client->adapter->algo->toggle_smbus_host_notify(client->adapter, > > > > + state); > > > > + i2c_unlock_adapter(client->adapter); > > > > + > > > > + return ret; > > > > +} > > > > +EXPORT_SYMBOL_GPL(i2c_smbus_toggle_host_notify); > > > > + > > > > > > As this is an SMBus feature, can't it go to drivers/i2c/i2c-smbus.c? > > > > I asked myself this question, and I figured it was not possible because > > i2c-smbus consists in the smbus_alert i2c driver. > > > > After re-reading the code, I see now that it could definitively goes > > there too. However, if we choose not to expose the toggle to the > > drivers, then there will be no need to make deep changes in i2c-smbus > > :). > > The smbus_alert i2c driver was just the most simple way to implement > the SMBus Alert feature. It's not an actual device driver, nothing to > be afraid of. Hehe. OK. > > > (...) > > Thanks for the review! > > You're welcome. I wish I could do more but -ENOTIME :/ No worries. That's already a lot. And from my side, I was quite silent because I just spent days of vacation without checking mails :) Cheers, Benjamin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
