| Message ID | 20230206082025.1992331-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [-next] power: supply: rt9471: fix using wrong ce_gpio in rt9471_probe() | expand |
On Mon, Feb 06, 2023 at 04:20:25PM +0800, Yang Yingliang wrote: Hi, YingLiang: Many thanks for the fix. My original thought is to remove ce_gpio in chip data and make it all by SW control only, not to control by HW pin. Could you help to send v2 patch to remove 'ce_gpio' in chip data? And for the macro IS_ERR(chip->ce_gpio), just change to IS_ERR(ce_gpio). In patch v2, you can add my Reviewed-by tag. Reviewed-by: ChiYuan Huang <cy_huang@richtek.com> Thanks. > Pass the correct 'ce_gpio' to IS_ERR(), and assign it to > the 'chip->ce_gpio', if devm_gpiod_get_optional() succeed. > > Fixes: 4a1a5f6781d8 ("power: supply: rt9471: Add Richtek RT9471 charger driver") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/power/supply/rt9471.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c > index 5d3cf375ad5c..de0bf484d313 100644 > --- a/drivers/power/supply/rt9471.c > +++ b/drivers/power/supply/rt9471.c > @@ -851,10 +851,12 @@ static int rt9471_probe(struct i2c_client *i2c) > > /* Default pull charge enable gpio to make 'CHG_EN' by SW control only */ > ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", GPIOD_OUT_HIGH); > - if (IS_ERR(chip->ce_gpio)) > + if (IS_ERR(ce_gpio)) > return dev_err_probe(dev, PTR_ERR(ce_gpio), > "Failed to config charge enable gpio\n"); > > + chip->ce_gpio = ce_gpio; > + > regmap = devm_regmap_init_i2c(i2c, &rt9471_regmap_config); > if (IS_ERR(regmap)) > return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n"); > -- > 2.25.1 >
Due to the Richtek email rule, YingLiang's mailbox could be blocked. Resend by my personal gmail. Hi, YingLiang: Many thanks for the fix. My original thought is to remove ce_gpio in chip data and make it all by SW control only, not to control by HW pin. Could you help to send v2 patch to remove 'ce_gpio' in chip data? And for the macro IS_ERR(chip->ce_gpio), just change to IS_ERR(ce_gpio). In patch v2, you can add my Reviewed-by tag. Reviewed-by: ChiYuan Huang <cy_huang@richtek.com> On Mon, Feb 06, 2023 at 04:20:25PM +0800, Yang Yingliang wrote: > Pass the correct 'ce_gpio' to IS_ERR(), and assign it to > the 'chip->ce_gpio', if devm_gpiod_get_optional() succeed. > > Fixes: 4a1a5f6781d8 ("power: supply: rt9471: Add Richtek RT9471 charger driver") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/power/supply/rt9471.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c > index 5d3cf375ad5c..de0bf484d313 100644 > --- a/drivers/power/supply/rt9471.c > +++ b/drivers/power/supply/rt9471.c > @@ -851,10 +851,12 @@ static int rt9471_probe(struct i2c_client *i2c) > > /* Default pull charge enable gpio to make 'CHG_EN' by SW control only */ > ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", GPIOD_OUT_HIGH); > - if (IS_ERR(chip->ce_gpio)) > + if (IS_ERR(ce_gpio)) > return dev_err_probe(dev, PTR_ERR(ce_gpio), > "Failed to config charge enable gpio\n"); > > + chip->ce_gpio = ce_gpio; > + > regmap = devm_regmap_init_i2c(i2c, &rt9471_regmap_config); > if (IS_ERR(regmap)) > return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n"); > -- > 2.25.1 >
Hi, On 2023/2/6 17:13, ChiYuan Huang wrote: > Due to the Richtek email rule, YingLiang's mailbox could be blocked. > Resend by my personal gmail. > > Hi, YingLiang: > > Many thanks for the fix. > > My original thought is to remove ce_gpio in chip data and make it > all by SW control only, not to control by HW pin. > > Could you help to send v2 patch to remove 'ce_gpio' in chip data? > And for the macro IS_ERR(chip->ce_gpio), just change to IS_ERR(ce_gpio). Did you mean to change the code like this: diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c index 5d3cf375ad5c..1ea40876494b 100644 --- a/drivers/power/supply/rt9471.c +++ b/drivers/power/supply/rt9471.c @@ -141,7 +141,6 @@ enum { struct rt9471_chip { struct device *dev; - struct gpio_desc *ce_gpio; struct regmap *regmap; struct regmap_field *rm_fields[F_MAX_FIELDS]; struct regmap_irq_chip_data *irq_chip_data; @@ -851,7 +850,7 @@ static int rt9471_probe(struct i2c_client *i2c) /* Default pull charge enable gpio to make 'CHG_EN' by SW control only */ ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", GPIOD_OUT_HIGH); - if (IS_ERR(chip->ce_gpio)) + if (IS_ERR(ce_gpio)) return dev_err_probe(dev, PTR_ERR(ce_gpio), "Failed to config charge enable gpio\n"); Thanks, Yang > > In patch v2, you can add my Reviewed-by tag. > Reviewed-by: ChiYuan Huang <cy_huang@richtek.com> > > On Mon, Feb 06, 2023 at 04:20:25PM +0800, Yang Yingliang wrote: >> Pass the correct 'ce_gpio' to IS_ERR(), and assign it to >> the 'chip->ce_gpio', if devm_gpiod_get_optional() succeed. >> >> Fixes: 4a1a5f6781d8 ("power: supply: rt9471: Add Richtek RT9471 charger driver") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> drivers/power/supply/rt9471.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c >> index 5d3cf375ad5c..de0bf484d313 100644 >> --- a/drivers/power/supply/rt9471.c >> +++ b/drivers/power/supply/rt9471.c >> @@ -851,10 +851,12 @@ static int rt9471_probe(struct i2c_client *i2c) >> >> /* Default pull charge enable gpio to make 'CHG_EN' by SW control only */ >> ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", GPIOD_OUT_HIGH); >> - if (IS_ERR(chip->ce_gpio)) >> + if (IS_ERR(ce_gpio)) >> return dev_err_probe(dev, PTR_ERR(ce_gpio), >> "Failed to config charge enable gpio\n"); >> >> + chip->ce_gpio = ce_gpio; >> + >> regmap = devm_regmap_init_i2c(i2c, &rt9471_regmap_config); >> if (IS_ERR(regmap)) >> return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n"); >> -- >> 2.25.1 >> > .
Yang Yingliang <yangyingliang@huawei.com> 於 2023年2月6日 週一 下午7:47寫道： > > Hi, > > On 2023/2/6 17:13, ChiYuan Huang wrote: > > Due to the Richtek email rule, YingLiang's mailbox could be blocked. > > Resend by my personal gmail. > > > > Hi, YingLiang: > > > > Many thanks for the fix. > > > > My original thought is to remove ce_gpio in chip data and make it > > all by SW control only, not to control by HW pin. > > > > Could you help to send v2 patch to remove 'ce_gpio' in chip data? > > And for the macro IS_ERR(chip->ce_gpio), just change to IS_ERR(ce_gpio). > Did you mean to change the code like this: Yes. Thank you. > > diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c > index 5d3cf375ad5c..1ea40876494b 100644 > --- a/drivers/power/supply/rt9471.c > +++ b/drivers/power/supply/rt9471.c > @@ -141,7 +141,6 @@ enum { > > struct rt9471_chip { > struct device *dev; > - struct gpio_desc *ce_gpio; > struct regmap *regmap; > struct regmap_field *rm_fields[F_MAX_FIELDS]; > struct regmap_irq_chip_data *irq_chip_data; > @@ -851,7 +850,7 @@ static int rt9471_probe(struct i2c_client *i2c) > > /* Default pull charge enable gpio to make 'CHG_EN' by SW > control only */ > ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", > GPIOD_OUT_HIGH); > - if (IS_ERR(chip->ce_gpio)) > + if (IS_ERR(ce_gpio)) > return dev_err_probe(dev, PTR_ERR(ce_gpio), > "Failed to config charge enable > gpio\n"); > > Thanks, > Yang > > > > In patch v2, you can add my Reviewed-by tag. > > Reviewed-by: ChiYuan Huang <cy_huang@richtek.com> > > > > On Mon, Feb 06, 2023 at 04:20:25PM +0800, Yang Yingliang wrote: > >> Pass the correct 'ce_gpio' to IS_ERR(), and assign it to > >> the 'chip->ce_gpio', if devm_gpiod_get_optional() succeed. > >> > >> Fixes: 4a1a5f6781d8 ("power: supply: rt9471: Add Richtek RT9471 charger driver") > >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > >> --- > >> drivers/power/supply/rt9471.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c > >> index 5d3cf375ad5c..de0bf484d313 100644 > >> --- a/drivers/power/supply/rt9471.c > >> +++ b/drivers/power/supply/rt9471.c > >> @@ -851,10 +851,12 @@ static int rt9471_probe(struct i2c_client *i2c) > >> > >> /* Default pull charge enable gpio to make 'CHG_EN' by SW control only */ > >> ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", GPIOD_OUT_HIGH); > >> - if (IS_ERR(chip->ce_gpio)) > >> + if (IS_ERR(ce_gpio)) > >> return dev_err_probe(dev, PTR_ERR(ce_gpio), > >> "Failed to config charge enable gpio\n"); > >> > >> + chip->ce_gpio = ce_gpio; > >> + > >> regmap = devm_regmap_init_i2c(i2c, &rt9471_regmap_config); > >> if (IS_ERR(regmap)) > >> return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n"); > >> -- > >> 2.25.1 > >> > > .
diff --git a/drivers/power/supply/rt9471.c b/drivers/power/supply/rt9471.c index 5d3cf375ad5c..de0bf484d313 100644 --- a/drivers/power/supply/rt9471.c +++ b/drivers/power/supply/rt9471.c @@ -851,10 +851,12 @@ static int rt9471_probe(struct i2c_client *i2c) /* Default pull charge enable gpio to make 'CHG_EN' by SW control only */ ce_gpio = devm_gpiod_get_optional(dev, "charge-enable", GPIOD_OUT_HIGH); - if (IS_ERR(chip->ce_gpio)) + if (IS_ERR(ce_gpio)) return dev_err_probe(dev, PTR_ERR(ce_gpio), "Failed to config charge enable gpio\n"); + chip->ce_gpio = ce_gpio; + regmap = devm_regmap_init_i2c(i2c, &rt9471_regmap_config); if (IS_ERR(regmap)) return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n");
Pass the correct 'ce_gpio' to IS_ERR(), and assign it to the 'chip->ce_gpio', if devm_gpiod_get_optional() succeed. Fixes: 4a1a5f6781d8 ("power: supply: rt9471: Add Richtek RT9471 charger driver") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/power/supply/rt9471.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)