| Message ID | 20190917144449.32739-1-m.felsch@pengutronix.de |
|---|---|
| Headers | show |
| Series | Add simple-pm ops | expand |
On 17/09/2019 16:44, Marco Felsch wrote: > Currently we don't handle the supply. We need to add the supply support > to be able to switch the supply off e.g. during a suspend-to-ram > operation. So we can guarantee a correct (re-)initialization. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> Hi, tested it on a i.MX6 based platform with an usb2512. Therefore please feel free to add: Reviewed-by: Richard Leitner <richard.leitner@skidata.com> and/or Tested-by: Richard Leitner <richard.leitner@skidata.com> regards;Richard.L > --- > drivers/usb/misc/usb251xb.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > index 6ca9111d150a..05819167604d 100644 > --- a/drivers/usb/misc/usb251xb.c > +++ b/drivers/usb/misc/usb251xb.c > @@ -17,6 +17,7 @@ > #include <linux/module.h> > #include <linux/nls.h> > #include <linux/of_device.h> > +#include <linux/regulator/consumer.h> > #include <linux/slab.h> > > /* Internal Register Set Addresses & Default Values acc. to DS00001692C */ > @@ -116,6 +117,7 @@ > struct usb251xb { > struct device *dev; > struct i2c_client *i2c; > + struct regulator *vdd; > u8 skip_config; > struct gpio_desc *gpio_reset; > u16 vendor_id; > @@ -420,6 +422,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub, > return err; > } > > + hub->vdd = devm_regulator_get(dev, "vdd"); > + if (IS_ERR(hub->vdd)) > + return PTR_ERR(hub->vdd); > + > if (of_property_read_u16_array(np, "vendor-id", &hub->vendor_id, 1)) > hub->vendor_id = USB251XB_DEF_VENDOR_ID; > > @@ -665,6 +671,10 @@ static int usb251xb_probe(struct usb251xb *hub) > if (err) > return err; > > + err = regulator_enable(hub->vdd); > + if (err) > + return err; > + > err = usb251xb_connect(hub); > if (err) { > dev_err(dev, "Failed to connect hub (%d)\n", err); >
On 17/09/2019 16:44, Marco Felsch wrote: > Currently the reset handler was always called to deassert the reset > line because assert the line was done during probe. Now if we want to > support pm by turn of the supply we need to call this routine twice and > the i2c_lock_bus is done twice too. To simplify that we can drop the > state and just do a reset in one go. So a future pm operation don't need > to lock the i2c bus twice. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> Hi, tested it on a i.MX6 based platform with an usb2512. Therefore please feel free to add: Reviewed-by: Richard Leitner <richard.leitner@skidata.com> and/or Tested-by: Richard Leitner <richard.leitner@skidata.com> regards;Richard.L > --- > drivers/usb/misc/usb251xb.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > index 05819167604d..bc031d33f433 100644 > --- a/drivers/usb/misc/usb251xb.c > +++ b/drivers/usb/misc/usb251xb.c > @@ -263,20 +263,19 @@ static int usb251x_check_gpio_chip(struct usb251xb *hub) > } > #endif > > -static void usb251xb_reset(struct usb251xb *hub, int state) > +static void usb251xb_reset(struct usb251xb *hub) > { > if (!hub->gpio_reset) > return; > > i2c_lock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); > > - gpiod_set_value_cansleep(hub->gpio_reset, state); > + gpiod_set_value_cansleep(hub->gpio_reset, 1); > + usleep_range(1, 10); /* >=1us RESET_N asserted */ > + gpiod_set_value_cansleep(hub->gpio_reset, 0); > > /* wait for hub recovery/stabilization */ > - if (!state) > - usleep_range(500, 750); /* >=500us at power on */ > - else > - usleep_range(1, 10); /* >=1us at power down */ > + usleep_range(500, 750); /* >=500us after RESET_N deasserted */ > > i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); > } > @@ -294,7 +293,7 @@ static int usb251xb_connect(struct usb251xb *hub) > i2c_wb[0] = 0x01; > i2c_wb[1] = USB251XB_STATUS_COMMAND_ATTACH; > > - usb251xb_reset(hub, 0); > + usb251xb_reset(hub); > > err = i2c_smbus_write_i2c_block_data(hub->i2c, > USB251XB_ADDR_STATUS_COMMAND, 2, i2c_wb); > @@ -344,7 +343,7 @@ static int usb251xb_connect(struct usb251xb *hub) > i2c_wb[USB251XB_ADDR_PORT_MAP_7] = hub->port_map7; > i2c_wb[USB251XB_ADDR_STATUS_COMMAND] = USB251XB_STATUS_COMMAND_ATTACH; > > - usb251xb_reset(hub, 0); > + usb251xb_reset(hub); > > /* write registers */ > for (i = 0; i < (USB251XB_I2C_REG_SZ / USB251XB_I2C_WRITE_SZ); i++) { >
On 17/09/2019 16:44, Marco Felsch wrote: > Currently the driver don't support pm_ops. These ops are not necessary > if the supply isn't switchable (always on). This assumptions seems to be > wrong because no one needs a powered hub during suspend-to-ram/disk. > > So adding simple_dev_pm_ops to be able to switch off the hub during > suspend and to restore the config after a resume operation. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> Hi, please feel free to add: Acked-by: Richard Leitner <richard.leitner@skidata.com> regards;Richard.L > --- > drivers/usb/misc/usb251xb.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > index bc031d33f433..5bba19937da1 100644 > --- a/drivers/usb/misc/usb251xb.c > +++ b/drivers/usb/misc/usb251xb.c > @@ -701,6 +701,29 @@ static int usb251xb_i2c_probe(struct i2c_client *i2c, > return usb251xb_probe(hub); > } > > +static int __maybe_unused usb251xb_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct usb251xb *hub = i2c_get_clientdata(client); > + > + return regulator_disable(hub->vdd); > +} > + > +static int __maybe_unused usb251xb_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct usb251xb *hub = i2c_get_clientdata(client); > + int err; > + > + err = regulator_enable(hub->vdd); > + if (err) > + return err; > + > + return usb251xb_connect(hub); > +} > + > +static SIMPLE_DEV_PM_OPS(usb251xb_pm_ops, usb251xb_suspend, usb251xb_resume); > + > static const struct i2c_device_id usb251xb_id[] = { > { "usb2512b", 0 }, > { "usb2512bi", 0 }, > @@ -718,6 +741,7 @@ static struct i2c_driver usb251xb_i2c_driver = { > .driver = { > .name = DRIVER_NAME, > .of_match_table = of_match_ptr(usb251xb_of_match), > + .pm = &usb251xb_pm_ops, > }, > .probe = usb251xb_i2c_probe, > .id_table = usb251xb_id, >