On Wed, Aug 19, 2015 at 9:02 AM, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > On Wed, Aug 19, 2015 at 03:44:49AM +0000, Gao Pandy wrote: >> From: Uwe Kleine-König <mailto:u.kleine-koenig@xxxxxxxxxxxxxx> Sent: Thursday, August 13, 2015 4:15 PM >> > > +static void i2c_imx_prepare_recovery(struct i2c_adapter *adap) { >> > > + struct imx_i2c_struct *i2c_imx; >> > > + >> > > + i2c_imx = container_of(adap, struct imx_i2c_struct, adapter); >> > > + if (i2c_imx->pins.sda && i2c_imx->pins.scl) { >> > > + pinctrl_pm_select_sleep_state(&adap->dev); >> > >> > Your requirement that the sleep state should configure the pins as gpio >> > is strange. Maybe better introduce a dedicated state for recovery? At >> > least you should document this requirement. >> >> In general, pinctrl sleep mode is gpio function. I will add this in binding doc. Thanks. > > Linus, do you have to say something here? It might be right to have the > gpio function as configuration for sleep mode, but it doesn't look right > for me to use this for recovery purposes. What it usually means is that the pin has a function mode such that an asynchronous edge detector is connected to it on the outer pad ring, maybe in tristate or some pull-up/down configuration. This makes is possible for the system to power off completely until an event occurs there with only some very peripheral electronics powered up. I think the terminology depend on the use case. See the section "GPIO mode pitfalls" in Documentation/pinctrl.txt If the use case is around the i2c traffic, it is a mode related to I2C, and if this mode is called "GPIO mode" in the data sheet is irrelevant, because it is obviously not used for the generic input/output but the specific I2C. The terminology should be made familiar to whoever needs to go in and read the code later. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
