Skip to contentSkip to conversation

gpio: sx150x: correct and refine reset-on-probe behavior.

PATCH In review Gregory Bean 2 participants 2 messages Aug 29, 2010 – Aug 30, 2010

Subsystems (2)
Arm Qualcomm mailing list GPIO
lore
Share & tools
lore

Gregory Bean

author
#0 lore raw

PATCH v2 1/1 gpio: sx150x: correct and refine reset-on-probe behavior.

Replace the arbitrary software-reset call from the device-probe method, because:
- It is defective. To work correctly, it should be two byte writes, not a single word write. As it stands, it does nothing.
- Some devices with sx150x expanders installed have their NRESET pins ganged on the same line, so resetting one causes the others to reset - not a nice thing to do arbitrarily!
- The probe, usually taking place at boot, implies a recent hard-reset, so a software reset at this point is just a waste of energy anyway.

Therefore, make it optional, defaulting to off, as this will match the common case of probing at powerup and also matches the current broken no-op behavior.

Cc Jean Delvare <[email protected]>
Signed-off-by Gregory Bean <[email protected]>
---
 drivers/gpio/sx150x.c      |   26 +++++++++++++++++++++-----
 include/linux/i2c/sx150x.h |    4 ++++
 2 files changed, 25 insertions(+), 5 deletions(-)
drivers/gpio/sx150x.c +21 -5
index b42f42c..823559a 100644
@@ -459,17 +459,33 @@ static int sx150x_init_io(struct sx150x_chip *chip, u8 base, u16 cfg)
 	return err;
 }
 
-static int sx150x_init_hw(struct sx150x_chip *chip,
-			struct sx150x_platform_data *pdata)
+static int sx150x_reset(struct sx150x_chip *chip)
 {
-	int err = 0;
+	int err;
 
-	err = i2c_smbus_write_word_data(chip->client,
+	err = i2c_smbus_write_byte_data(chip->client,
 					chip->dev_cfg->reg_reset,
-					0x3412);
+					0x12);
 	if (err < 0)
 		return err;
 
+	err = i2c_smbus_write_byte_data(chip->client,
+					chip->dev_cfg->reg_reset,
+					0x34);
+	return err;
+}
+
+static int sx150x_init_hw(struct sx150x_chip *chip,
+			struct sx150x_platform_data *pdata)
+{
+	int err = 0;
+
+	if (pdata->reset_during_probe) {
+		err = sx150x_reset(chip);
+		if (err < 0)
+			return err;
+	}
+
 	err = sx150x_i2c_write(chip->client,
 			chip->dev_cfg->reg_misc,
 			0x01);
include/linux/i2c/sx150x.h +4 -0
index ee3049c..52baa79 100644
@@ -63,6 +63,9 @@
  *            IRQ lines will appear.  Similarly to gpio_base, the expander
  *            will create a block of irqs beginning at this number.
  *            This value is ignored if irq_summary is < 0.
+ * @reset_during_probe: If set to true, the driver will trigger a full
+ *                      reset of the chip@the beginning of the probe
+ *                      in order to place it in a known state.
  */
 struct sx150x_platform_data {
 	unsigned gpio_base;

@@ -73,6 +76,7 @@ struct sx150x_platform_data {
 	u16      io_polarity;
 	int      irq_summary;
 	unsigned irq_base;
+	bool     reset_during_probe;
 };
 
 #endif /* __LINUX_I2C_SX150X_H */
Signature · 5 lines
1.7.0.4

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Reviewed-by on v2 1/1
#1 lore raw
Quoted 10 lines · Gregory Bean

Replace the arbitrary software-reset call from the device-probe method, because:
- It is defective. To work correctly, it should be two byte writes, not a single word write. As it stands, it does nothing.
- Some devices with sx150x expanders installed have their NRESET pins ganged on the same line, so resetting one causes the others to reset - not a nice thing to do arbitrarily!
- The probe, usually taking place at boot, implies a recent hard-reset, so a software reset at this point is just a waste of energy anyway.

Quoted 6 lines

Therefore, make it optional, defaulting to off, as this will match the common case of probing at powerup and also matches the current broken no-op behavior.

Cc Jean Delvare <[email protected]>
Signed-off-by Gregory Bean <[email protected]>
Reviewed-by Jean Delvare <[email protected]>
Quoted 70 lines
---
 drivers/gpio/sx150x.c      |   26 +++++++++++++++++++++-----
 include/linux/i2c/sx150x.h |    4 ++++
 2 files changed, 25 insertions(+), 5 deletions(-)
drivers/gpio/sx150x.c +21 -5
index b42f42c..823559a 100644
@@ -459,17 +459,33 @@ static int sx150x_init_io(struct sx150x_chip *chip, u8 base, u16 cfg)
 	return err;
 }
 
-static int sx150x_init_hw(struct sx150x_chip *chip,
-			struct sx150x_platform_data *pdata)
+static int sx150x_reset(struct sx150x_chip *chip)
 {
-	int err = 0;
+	int err;
 
-	err = i2c_smbus_write_word_data(chip->client,
+	err = i2c_smbus_write_byte_data(chip->client,
 					chip->dev_cfg->reg_reset,
-					0x3412);
+					0x12);
 	if (err < 0)
 		return err;
 
+	err = i2c_smbus_write_byte_data(chip->client,
+					chip->dev_cfg->reg_reset,
+					0x34);
+	return err;
+}
+
+static int sx150x_init_hw(struct sx150x_chip *chip,
+			struct sx150x_platform_data *pdata)
+{
+	int err = 0;
+
+	if (pdata->reset_during_probe) {
+		err = sx150x_reset(chip);
+		if (err < 0)
+			return err;
+	}
+
 	err = sx150x_i2c_write(chip->client,
 			chip->dev_cfg->reg_misc,
 			0x01);
include/linux/i2c/sx150x.h +4 -0
index ee3049c..52baa79 100644
@@ -63,6 +63,9 @@
  *            IRQ lines will appear.  Similarly to gpio_base, the expander
  *            will create a block of irqs beginning at this number.
  *            This value is ignored if irq_summary is < 0.
+ * @reset_during_probe: If set to true, the driver will trigger a full
+ *                      reset of the chip at the beginning of the probe
+ *                      in order to place it in a known state.
  */
 struct sx150x_platform_data {
 	unsigned gpio_base;

@@ -73,6 +76,7 @@ struct sx150x_platform_data {
 	u16      io_polarity;
 	int      irq_summary;
 	unsigned irq_base;
+	bool     reset_during_probe;
 };
 
 #endif /* __LINUX_I2C_SX150X_H */
Signature · 1 line
Jean Delvare