From 9b9f2b8bc2ac98d91da714660c53d1cdac999e09 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:01 +0800 Subject: [PATCH 01/13] i2c: i2c-smbus: Use threaded irq for smbalert Prior to this commit the smbalert_irq was handling in the hard irq context. This change switch to using a thread irq which avoids the need for the work thread. Using threaded irq also removes the need for the edge_triggered flag as the enabling / disabling of the hard irq for level triggered interrupts will be handled by the irq core. Without this change have an irq connected to something like an i2c gpio resulted in a null ptr deferences. Specifically handle_nested_irq calls the threaded irq handler. There are currently 3 in tree drivers affected by this change. i2c-parport driver calls i2c_handle_smbus_alert in a hard irq context. This driver use edge trigger interrupts which skip the enable / disable calls. But it still need to handle the smbus transaction on a thread. So the work thread is kept for this driver. i2c-parport-light & i2c-thunderx-pcidrv provide the irq number in the setup which will result in the thread irq being used. i2c-parport-light is edge trigger so the enable / disable call was skipped as well. i2c-thunderx-pcidrv is getting the edge / level trigger setting from of data and was setting the flag as required. However the irq core should handle this automatically. Signed-off-by: Phil Reid Reviewed-by: Benjamin Tissoires Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-parport-light.c | 1 - drivers/i2c/busses/i2c-parport.c | 1 - drivers/i2c/busses/i2c-thunderx-pcidrv.c | 6 ---- drivers/i2c/i2c-smbus.c | 41 ++++++++++-------------- include/linux/i2c-smbus.h | 1 - 5 files changed, 17 insertions(+), 33 deletions(-) diff --git a/drivers/i2c/busses/i2c-parport-light.c b/drivers/i2c/busses/i2c-parport-light.c index faa8fb8f2b8f..fa41ff799533 100644 --- a/drivers/i2c/busses/i2c-parport-light.c +++ b/drivers/i2c/busses/i2c-parport-light.c @@ -123,7 +123,6 @@ static struct i2c_adapter parport_adapter = { /* SMBus alert support */ static struct i2c_smbus_alert_setup alert_data = { - .alert_edge_triggered = 1, }; static struct i2c_client *ara; static struct lineop parport_ctrl_irq = { diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c index a8e54df4aed6..319209a07353 100644 --- a/drivers/i2c/busses/i2c-parport.c +++ b/drivers/i2c/busses/i2c-parport.c @@ -237,7 +237,6 @@ static void i2c_parport_attach(struct parport *port) /* Setup SMBus alert if supported */ if (adapter_parm[type].smbus_alert) { - adapter->alert_data.alert_edge_triggered = 1; adapter->ara = i2c_setup_smbus_alert(&adapter->adapter, &adapter->alert_data); if (adapter->ara) diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c index df0976f4432a..1a7cad874756 100644 --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c @@ -118,8 +118,6 @@ static void thunder_i2c_clock_disable(struct device *dev, struct clk *clk) static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c, struct device_node *node) { - u32 type; - if (!node) return -EINVAL; @@ -127,10 +125,6 @@ static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c, if (!i2c->alert_data.irq) return -EINVAL; - type = irqd_get_trigger_type(irq_get_irq_data(i2c->alert_data.irq)); - i2c->alert_data.alert_edge_triggered = - (type & IRQ_TYPE_LEVEL_MASK) ? 1 : 0; - i2c->ara = i2c_setup_smbus_alert(&i2c->adap, &i2c->alert_data); if (!i2c->ara) return -ENODEV; diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index f9271c713d20..d4af2701ac6e 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -25,8 +25,6 @@ #include struct i2c_smbus_alert { - unsigned int alert_edge_triggered:1; - int irq; struct work_struct alert; struct i2c_client *ara; /* Alert response address */ }; @@ -72,13 +70,12 @@ static int smbus_do_alert(struct device *dev, void *addrp) * The alert IRQ handler needs to hand work off to a task which can issue * SMBus calls, because those sleeping calls can't be made in IRQ context. */ -static void smbus_alert(struct work_struct *work) +static irqreturn_t smbus_alert(int irq, void *d) { - struct i2c_smbus_alert *alert; + struct i2c_smbus_alert *alert = d; struct i2c_client *ara; unsigned short prev_addr = 0; /* Not a valid address */ - alert = container_of(work, struct i2c_smbus_alert, alert); ara = alert->ara; for (;;) { @@ -115,21 +112,17 @@ static void smbus_alert(struct work_struct *work) prev_addr = data.addr; } - /* We handled all alerts; re-enable level-triggered IRQs */ - if (!alert->alert_edge_triggered) - enable_irq(alert->irq); + return IRQ_HANDLED; } -static irqreturn_t smbalert_irq(int irq, void *d) +static void smbalert_work(struct work_struct *work) { - struct i2c_smbus_alert *alert = d; + struct i2c_smbus_alert *alert; - /* Disable level-triggered IRQs until we handle them */ - if (!alert->alert_edge_triggered) - disable_irq_nosync(irq); + alert = container_of(work, struct i2c_smbus_alert, alert); + + smbus_alert(0, alert); - schedule_work(&alert->alert); - return IRQ_HANDLED; } /* Setup SMBALERT# infrastructure */ @@ -139,28 +132,28 @@ static int smbalert_probe(struct i2c_client *ara, struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev); struct i2c_smbus_alert *alert; struct i2c_adapter *adapter = ara->adapter; - int res; + int res, irq; alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert), GFP_KERNEL); if (!alert) return -ENOMEM; - alert->alert_edge_triggered = setup->alert_edge_triggered; - alert->irq = setup->irq; - INIT_WORK(&alert->alert, smbus_alert); + irq = setup->irq; + INIT_WORK(&alert->alert, smbalert_work); alert->ara = ara; - if (setup->irq > 0) { - res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq, - 0, "smbus_alert", alert); + if (irq > 0) { + res = devm_request_threaded_irq(&ara->dev, irq, + NULL, smbus_alert, + IRQF_SHARED | IRQF_ONESHOT, + "smbus_alert", alert); if (res) return res; } i2c_set_clientdata(ara, alert); - dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n", - setup->alert_edge_triggered ? "edge" : "level"); + dev_info(&adapter->dev, "supports SMBALERT#\n"); return 0; } diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h index a1385023a29b..19efbd14e812 100644 --- a/include/linux/i2c-smbus.h +++ b/include/linux/i2c-smbus.h @@ -42,7 +42,6 @@ * properly set. */ struct i2c_smbus_alert_setup { - unsigned int alert_edge_triggered:1; int irq; }; From 3c0a60bee1d157d4ed9ae6a9727d508158d002c7 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:02 +0800 Subject: [PATCH 02/13] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus In preparation to adding of_i2c_setup_smbus_alert() move i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert() will call i2c_setup_smbus_alert() and this avoid module dependecy issues. Signed-off-by: Phil Reid Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-smbus.c | 33 +++++++++++++++++++++++++++++++++ drivers/i2c/i2c-smbus.c | 32 -------------------------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index 10f00a82ec9d..7f3ec02f085a 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -17,6 +17,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include @@ -592,3 +593,35 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client, return i; } EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data_or_emulated); + +/** + * i2c_setup_smbus_alert - Setup SMBus alert support + * @adapter: the target adapter + * @setup: setup data for the SMBus alert handler + * Context: can sleep + * + * Setup handling of the SMBus alert protocol on a given I2C bus segment. + * + * Handling can be done either through our IRQ handler, or by the + * adapter (from its handler, periodic polling, or whatever). + * + * NOTE that if we manage the IRQ, we *MUST* know if it's level or + * edge triggered in order to hand it to the workqueue correctly. + * If triggering the alert seems to wedge the system, you probably + * should have said it's level triggered. + * + * This returns the ara client, which should be saved for later use with + * i2c_handle_smbus_alert() and ultimately i2c_unregister_device(); or NULL + * to indicate an error. + */ +struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter, + struct i2c_smbus_alert_setup *setup) +{ + struct i2c_board_info ara_board_info = { + I2C_BOARD_INFO("smbus_alert", 0x0c), + .platform_data = setup, + }; + + return i2c_new_device(adapter, &ara_board_info); +} +EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert); diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index d4af2701ac6e..d0bb0358b578 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -182,38 +182,6 @@ static struct i2c_driver smbalert_driver = { .id_table = smbalert_ids, }; -/** - * i2c_setup_smbus_alert - Setup SMBus alert support - * @adapter: the target adapter - * @setup: setup data for the SMBus alert handler - * Context: can sleep - * - * Setup handling of the SMBus alert protocol on a given I2C bus segment. - * - * Handling can be done either through our IRQ handler, or by the - * adapter (from its handler, periodic polling, or whatever). - * - * NOTE that if we manage the IRQ, we *MUST* know if it's level or - * edge triggered in order to hand it to the workqueue correctly. - * If triggering the alert seems to wedge the system, you probably - * should have said it's level triggered. - * - * This returns the ara client, which should be saved for later use with - * i2c_handle_smbus_alert() and ultimately i2c_unregister_device(); or NULL - * to indicate an error. - */ -struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter, - struct i2c_smbus_alert_setup *setup) -{ - struct i2c_board_info ara_board_info = { - I2C_BOARD_INFO("smbus_alert", 0x0c), - .platform_data = setup, - }; - - return i2c_new_device(adapter, &ara_board_info); -} -EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert); - /** * i2c_handle_smbus_alert - Handle an SMBus alert * @ara: the ARA client on the relevant adapter From 69d17246ab255dda8e71c8d65396b4aa6121b7ad Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:03 +0800 Subject: [PATCH 03/13] i2c: i2c-smbus: add of_i2c_setup_smbus_alert This commit adds of_i2c_setup_smbus_alert which allows the smbalert driver to be attached to an i2c adapter via the device tree. Signed-off-by: Phil Reid Acked-by: Rob Herring Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++-- drivers/i2c/i2c-core-smbus.c | 22 +++++++++++++++++++ drivers/i2c/i2c-smbus.c | 10 ++++++++- include/linux/i2c-smbus.h | 9 ++++++++ 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt index cee9d5055fa2..11263982470e 100644 --- a/Documentation/devicetree/bindings/i2c/i2c.txt +++ b/Documentation/devicetree/bindings/i2c/i2c.txt @@ -59,8 +59,8 @@ wants to support one of the below features, it should adapt the bindings below. interrupts used by the device. - interrupt-names - "irq" and "wakeup" names are recognized by I2C core, other names are - left to individual drivers. + "irq", "wakeup" and "smbus_alert" names are recognized by I2C core, + other names are left to individual drivers. - host-notify device uses SMBus host notify protocol instead of interrupt line. diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index 7f3ec02f085a..4bb9927afd01 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -625,3 +625,25 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter, return i2c_new_device(adapter, &ara_board_info); } EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert); + +#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF) +int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter) +{ + struct i2c_client *client; + int irq; + + irq = of_property_match_string(adapter->dev.of_node, "interrupt-names", + "smbus_alert"); + if (irq == -EINVAL || irq == -ENODATA) + return 0; + else if (irq < 0) + return irq; + + client = i2c_setup_smbus_alert(adapter, NULL); + if (!client) + return -ENODEV; + + return 0; +} +EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert); +#endif diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index d0bb0358b578..5a1dd7f13bac 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -139,7 +140,14 @@ static int smbalert_probe(struct i2c_client *ara, if (!alert) return -ENOMEM; - irq = setup->irq; + if (setup) { + irq = setup->irq; + } else { + irq = of_irq_get_byname(adapter->dev.of_node, "smbus_alert"); + if (irq <= 0) + return irq; + } + INIT_WORK(&alert->alert, smbalert_work); alert->ara = ara; diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h index 19efbd14e812..fb0e040b1abb 100644 --- a/include/linux/i2c-smbus.h +++ b/include/linux/i2c-smbus.h @@ -49,4 +49,13 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter, struct i2c_smbus_alert_setup *setup); int i2c_handle_smbus_alert(struct i2c_client *ara); +#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF) +int of_i2c_setup_smbus_alert(struct i2c_adapter *adap); +#else +static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap) +{ + return 0; +} +#endif + #endif /* _LINUX_I2C_SMBUS_H */ From f8756c67b3de71a46255aa478ab590ff28e60ac9 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:04 +0800 Subject: [PATCH 04/13] i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter Add a call to of_i2c_setup_smbus_alert when a i2c adapter is registered so the the smbalert driver can be registered. Signed-off-by: Phil Reid Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core-base.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 56e46581b84b..130d22bed563 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -1259,6 +1260,10 @@ static int i2c_register_adapter(struct i2c_adapter *adap) goto out_list; } + res = of_i2c_setup_smbus_alert(adap); + if (res) + goto out_reg; + dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name); pm_runtime_no_callbacks(&adap->dev); @@ -1290,6 +1295,10 @@ static int i2c_register_adapter(struct i2c_adapter *adap) return 0; +out_reg: + init_completion(&adap->dev_released); + device_unregister(&adap->dev); + wait_for_completion(&adap->dev_released); out_list: mutex_lock(&core_lock); idr_remove(&i2c_adapter_idr, adap->nr); From 148baf1ddfb65019ee510ba2a76c2e7cd9752982 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:05 +0800 Subject: [PATCH 05/13] i2c: mux: pca954x: call request irq after adding mux segments The pca954x device do not have the ability to mask interrupts. For i2c slave devices that also don't have masking ability (eg ltc1760 smbalert output) delay registering the irq until after the mux segments have been configured. During the mux add_adaptor call the core i2c system can register an smbalert handler which would then be called immediately when the irq is registered. This smbalert handler will then clear the pending irq. This removes the need for the irq_mask / irq_unmask calls that were original used to do this. Signed-off-by: Phil Reid Acked-by: Peter Rosin Signed-off-by: Wolfram Sang --- drivers/i2c/muxes/i2c-mux-pca954x.c | 93 +++++++++-------------------- 1 file changed, 29 insertions(+), 64 deletions(-) diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c index 7b992db38021..14aee65298f7 100644 --- a/drivers/i2c/muxes/i2c-mux-pca954x.c +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c @@ -246,36 +246,6 @@ static irqreturn_t pca954x_irq_handler(int irq, void *dev_id) return handled ? IRQ_HANDLED : IRQ_NONE; } -static void pca954x_irq_mask(struct irq_data *idata) -{ - struct pca954x *data = irq_data_get_irq_chip_data(idata); - unsigned int pos = idata->hwirq; - unsigned long flags; - - raw_spin_lock_irqsave(&data->lock, flags); - - data->irq_mask &= ~BIT(pos); - if (!data->irq_mask) - disable_irq(data->client->irq); - - raw_spin_unlock_irqrestore(&data->lock, flags); -} - -static void pca954x_irq_unmask(struct irq_data *idata) -{ - struct pca954x *data = irq_data_get_irq_chip_data(idata); - unsigned int pos = idata->hwirq; - unsigned long flags; - - raw_spin_lock_irqsave(&data->lock, flags); - - if (!data->irq_mask) - enable_irq(data->client->irq); - data->irq_mask |= BIT(pos); - - raw_spin_unlock_irqrestore(&data->lock, flags); -} - static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type) { if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW) @@ -285,8 +255,6 @@ static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type) static struct irq_chip pca954x_irq_chip = { .name = "i2c-mux-pca954x", - .irq_mask = pca954x_irq_mask, - .irq_unmask = pca954x_irq_unmask, .irq_set_type = pca954x_irq_set_type, }; @@ -294,7 +262,7 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc) { struct pca954x *data = i2c_mux_priv(muxc); struct i2c_client *client = data->client; - int c, err, irq; + int c, irq; if (!data->chip->has_irq || client->irq <= 0) return 0; @@ -314,24 +282,22 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc) handle_simple_irq); } - err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL, - pca954x_irq_handler, - IRQF_ONESHOT | IRQF_SHARED, - "pca954x", data); - if (err) - goto err_req_irq; - - disable_irq(data->client->irq); - return 0; -err_req_irq: - for (c = 0; c < data->chip->nchans; c++) { - irq = irq_find_mapping(data->irq, c); - irq_dispose_mapping(irq); - } - irq_domain_remove(data->irq); +} - return err; +static void pca954x_cleanup(struct i2c_mux_core *muxc) +{ + struct pca954x *data = i2c_mux_priv(muxc); + int c, irq; + + if (data->irq) { + for (c = 0; c < data->chip->nchans; c++) { + irq = irq_find_mapping(data->irq, c); + irq_dispose_mapping(irq); + } + irq_domain_remove(data->irq); + } + i2c_mux_del_adapters(muxc); } /* @@ -391,7 +357,7 @@ static int pca954x_probe(struct i2c_client *client, ret = pca954x_irq_setup(muxc); if (ret) - goto fail_del_adapters; + goto fail_cleanup; /* Now create an adapter for each channel */ for (num = 0; num < data->chip->nchans; num++) { @@ -414,7 +380,16 @@ static int pca954x_probe(struct i2c_client *client, ret = i2c_mux_add_adapter(muxc, force, num, class); if (ret) - goto fail_del_adapters; + goto fail_cleanup; + } + + if (data->irq) { + ret = devm_request_threaded_irq(&client->dev, data->client->irq, + NULL, pca954x_irq_handler, + IRQF_ONESHOT | IRQF_SHARED, + "pca954x", data); + if (ret) + goto fail_cleanup; } dev_info(&client->dev, @@ -424,26 +399,16 @@ static int pca954x_probe(struct i2c_client *client, return 0; -fail_del_adapters: - i2c_mux_del_adapters(muxc); +fail_cleanup: + pca954x_cleanup(muxc); return ret; } static int pca954x_remove(struct i2c_client *client) { struct i2c_mux_core *muxc = i2c_get_clientdata(client); - struct pca954x *data = i2c_mux_priv(muxc); - int c, irq; - if (data->irq) { - for (c = 0; c < data->chip->nchans; c++) { - irq = irq_find_mapping(data->irq, c); - irq_dispose_mapping(irq); - } - irq_domain_remove(data->irq); - } - - i2c_mux_del_adapters(muxc); + pca954x_cleanup(muxc); return 0; } From e460617e85db8af533c9f70c9ab94d4fa111b379 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:06 +0800 Subject: [PATCH 06/13] i2c: mux: pca954x: Return error if irq_create_mapping fails irq_create_mapping can return an error, report error to log and return. Cleanup will occur in the probe function when an error is returned. Suggested-by: Peter Rosin Acked-by: Peter Rosin Signed-off-by: Phil Reid Signed-off-by: Wolfram Sang --- drivers/i2c/muxes/i2c-mux-pca954x.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c index 14aee65298f7..2ca068d8b92d 100644 --- a/drivers/i2c/muxes/i2c-mux-pca954x.c +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c @@ -277,6 +277,10 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc) for (c = 0; c < data->chip->nchans; c++) { irq = irq_create_mapping(data->irq, c); + if (!irq) { + dev_err(&client->dev, "failed irq create map\n"); + return -EINVAL; + } irq_set_chip_data(irq, data); irq_set_chip_and_handler(irq, &pca954x_irq_chip, handle_simple_irq); From 26b61a652f6d52ec9c14f6be7c5a854a4fe831ae Mon Sep 17 00:00:00 2001 From: Karl-Heinz Schneider Date: Thu, 24 Aug 2017 17:31:07 +0800 Subject: [PATCH 07/13] Documentation: Add sbs-manager device tree node documentation This patch adds device tree documentation for the sbs-manager Signed-off-by: Karl-Heinz Schneider Signed-off-by: Phil Reid Acked-by: Rob Herring Reviewed-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- .../bindings/power/supply/sbs,sbs-manager.txt | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt diff --git a/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt new file mode 100644 index 000000000000..4b2195571a49 --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt @@ -0,0 +1,66 @@ +Binding for sbs-manager + +Required properties: +- compatible: ",", "sbs,sbs-charger" as fallback. The part + number compatible string might be used in order to take care of vendor + specific registers. +- reg: integer, i2c address of the device. Should be <0xa>. +Optional properties: +- gpio-controller: Marks the port as GPIO controller. + See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt. +- #gpio-cells: Should be <2>. The first cell is the pin number, the second cell + is used to specify optional parameters: + See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt. + +From OS view the device is basically an i2c-mux used to communicate with up to +four smart battery devices at address 0xb. The driver actually implements this +behaviour. So standard i2c-mux nodes can be used to register up to four slave +batteries. Channels will be numerated starting from 1 to 4. + +Example: + +batman@a { + compatible = "lltc,ltc1760", "sbs,sbs-manager"; + reg = <0x0a>; + #address-cells = <1>; + #size-cells = <0>; + + gpio-controller; + #gpio-cells = <2>; + + i2c@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + battery@b { + compatible = "ti,bq2060", "sbs,sbs-battery"; + reg = <0x0b>; + sbs,battery-detect-gpios = <&batman 1 1>; + }; + }; + + i2c@2 { + #address-cells = <1>; + #size-cells = <0>; + reg = <2>; + + battery@b { + compatible = "ti,bq2060", "sbs,sbs-battery"; + reg = <0x0b>; + sbs,battery-detect-gpios = <&batman 2 1>; + }; + }; + + i2c@3 { + #address-cells = <1>; + #size-cells = <0>; + reg = <3>; + + battery@b { + compatible = "ti,bq2060", "sbs,sbs-battery"; + reg = <0x0b>; + sbs,battery-detect-gpios = <&batman 3 1>; + }; + }; +}; From dbc4deda03fe61a1c29d8218269714bf2c334b9b Mon Sep 17 00:00:00 2001 From: Karl-Heinz Schneider Date: Thu, 24 Aug 2017 17:31:08 +0800 Subject: [PATCH 08/13] power: Adds support for Smart Battery System Manager This patch adds support for Smart Battery System Manager. A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of communicating up to four I2C smart battery devices. All smart battery devices are listening at address 0x0b, so the SBSM muliplexes between them. The driver makes use of the I2C-Mux framework to allow smart batteries to be bound via device tree, i.e. the sbs-battery driver. Via sysfs interface the online state and charge type are presented. If the driver is bound as ltc1760 (an implementation of a Dual Smart Battery System Manager) the charge type can also be changed from trickle to fast. Signed-off-by: Karl-Heinz Schneider Signed-off-by: Phil Reid Reviewed-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- drivers/power/supply/Kconfig | 13 ++ drivers/power/supply/Makefile | 1 + drivers/power/supply/sbs-manager.c | 323 +++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+) create mode 100644 drivers/power/supply/sbs-manager.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index 5ab90c1f3f7c..1899b3ec6bec 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -184,6 +184,19 @@ config CHARGER_SBS help Say Y to include support for SBS compilant battery chargers. +config MANAGER_SBS + tristate "Smart Battery System Manager" + depends on I2C && I2C_MUX + help + Say Y here to include support for Smart Battery System Manager + ICs. The driver reports online and charging status via sysfs. + It presents itself also as I2C mux which allows to bind + smart battery driver to its ports. + Supported is for example LTC1760. + + This driver can also be built as a module. If so, the module will be + called sbs-manager. + config BATTERY_BQ27XXX tristate "BQ27xxx battery driver" help diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index 621a19058fec..3e8f49a17770 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o obj-$(CONFIG_BATTERY_SBS) += sbs-battery.o obj-$(CONFIG_CHARGER_SBS) += sbs-charger.o +obj-$(CONFIG_MANAGER_SBS) += sbs-manager.o obj-$(CONFIG_BATTERY_BQ27XXX) += bq27xxx_battery.o obj-$(CONFIG_BATTERY_BQ27XXX_I2C) += bq27xxx_battery_i2c.o obj-$(CONFIG_BATTERY_BQ27XXX_HDQ) += bq27xxx_battery_hdq.o diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c new file mode 100644 index 000000000000..47c09a178411 --- /dev/null +++ b/drivers/power/supply/sbs-manager.c @@ -0,0 +1,323 @@ +/* + * Driver for SBS compliant Smart Battery System Managers + * + * The device communicates via i2c at address 0x0a and multiplexes access to up + * to four smart batteries at address 0x0b. + * + * Via sysfs interface the online state and charge type are presented. + * + * Datasheet SBSM: http://sbs-forum.org/specs/sbsm100b.pdf + * Datasheet LTC1760: http://cds.linear.com/docs/en/datasheet/1760fb.pdf + * + * Karl-Heinz Schneider + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include + +#define SBSM_MAX_BATS 4 +#define SBSM_RETRY_CNT 3 + +/* registers addresses */ +#define SBSM_CMD_BATSYSSTATE 0x01 +#define SBSM_CMD_BATSYSSTATECONT 0x02 +#define SBSM_CMD_BATSYSINFO 0x04 +#define SBSM_CMD_LTC 0x3c + +#define SBSM_MASK_BAT_SUPPORTED GENMASK(3, 0) +#define SBSM_MASK_CHARGE_BAT GENMASK(7, 4) +#define SBSM_BIT_AC_PRESENT BIT(0) +#define SBSM_BIT_TURBO BIT(7) + +#define SBSM_SMB_BAT_OFFSET 11 +struct sbsm_data { + struct i2c_client *client; + struct i2c_mux_core *muxc; + + struct power_supply *psy; + + u8 cur_chan; /* currently selected channel */ + bool is_ltc1760; /* special capabilities */ +}; + +static enum power_supply_property sbsm_props[] = { + POWER_SUPPLY_PROP_ONLINE, + POWER_SUPPLY_PROP_CHARGE_TYPE, +}; + +static int sbsm_read_word(struct i2c_client *client, u8 address) +{ + int reg, retries; + + for (retries = SBSM_RETRY_CNT; retries > 0; retries--) { + reg = i2c_smbus_read_word_data(client, address); + if (reg >= 0) + break; + } + + if (reg < 0) { + dev_err(&client->dev, "failed to read register 0x%02x\n", + address); + } + + return reg; +} + +static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word) +{ + int ret, retries; + + for (retries = SBSM_RETRY_CNT; retries > 0; retries--) { + ret = i2c_smbus_write_word_data(client, address, word); + if (ret >= 0) + break; + } + if (ret < 0) + dev_err(&client->dev, "failed to write to register 0x%02x\n", + address); + + return ret; +} + +static int sbsm_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct sbsm_data *data = power_supply_get_drvdata(psy); + int regval = 0; + + switch (psp) { + case POWER_SUPPLY_PROP_ONLINE: + regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT); + if (regval < 0) + return regval; + val->intval = !!(regval & SBSM_BIT_AC_PRESENT); + break; + + case POWER_SUPPLY_PROP_CHARGE_TYPE: + regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE); + if (regval < 0) + return regval; + + if ((regval & SBSM_MASK_CHARGE_BAT) == 0) { + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE; + return 0; + } + val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE; + + if (data->is_ltc1760) { + /* charge mode fast if turbo is active */ + regval = sbsm_read_word(data->client, SBSM_CMD_LTC); + if (regval < 0) + return regval; + else if (regval & SBSM_BIT_TURBO) + val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST; + } + break; + + default: + return -EINVAL; + } + + return 0; +} + +static int sbsm_prop_is_writeable(struct power_supply *psy, + enum power_supply_property psp) +{ + struct sbsm_data *data = power_supply_get_drvdata(psy); + + return (psp == POWER_SUPPLY_PROP_CHARGE_TYPE) && data->is_ltc1760; +} + +static int sbsm_set_property(struct power_supply *psy, + enum power_supply_property psp, + const union power_supply_propval *val) +{ + struct sbsm_data *data = power_supply_get_drvdata(psy); + int ret = -EINVAL; + u16 regval; + + switch (psp) { + case POWER_SUPPLY_PROP_CHARGE_TYPE: + /* write 1 to TURBO if type fast is given */ + if (!data->is_ltc1760) + break; + regval = val->intval == + POWER_SUPPLY_CHARGE_TYPE_FAST ? SBSM_BIT_TURBO : 0; + ret = sbsm_write_word(data->client, SBSM_CMD_LTC, regval); + break; + + default: + break; + } + + return ret; +} + +/* + * Switch to battery + * Parameter chan is directly the content of SMB_BAT* nibble + */ +static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) +{ + struct sbsm_data *data = i2c_mux_priv(muxc); + struct device *dev = &data->client->dev; + int ret = 0; + u16 reg; + + if (data->cur_chan == chan) + return ret; + + /* chan goes from 1 ... 4 */ + reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan); + ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg); + if (ret) + dev_err(dev, "Failed to select channel %i\n", chan); + else + data->cur_chan = chan; + + return ret; +} + +static const struct power_supply_desc sbsm_default_psy_desc = { + .type = POWER_SUPPLY_TYPE_MAINS, + .properties = sbsm_props, + .num_properties = ARRAY_SIZE(sbsm_props), + .get_property = &sbsm_get_property, + .set_property = &sbsm_set_property, + .property_is_writeable = &sbsm_prop_is_writeable, +}; + +static int sbsm_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); + struct sbsm_data *data; + struct device *dev = &client->dev; + struct power_supply_desc *psy_desc; + struct power_supply_config psy_cfg = {}; + int ret = 0, i, supported_bats; + + /* Device listens only at address 0x0a */ + if (client->addr != 0x0a) + return -EINVAL; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) + return -EPFNOSUPPORT; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + i2c_set_clientdata(client, data); + + data->client = client; + data->is_ltc1760 = !!strstr(id->name, "ltc1760"); + + ret = sbsm_read_word(client, SBSM_CMD_BATSYSINFO); + if (ret < 0) + return ret; + supported_bats = ret & SBSM_MASK_BAT_SUPPORTED; + + data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0, + I2C_MUX_LOCKED, &sbsm_select, NULL); + if (!data->muxc) { + dev_err(dev, "failed to alloc i2c mux\n"); + ret = -ENOMEM; + goto err_mux_alloc; + } + data->muxc->priv = data; + + /* register muxed i2c channels. One for each supported battery */ + for (i = 0; i < SBSM_MAX_BATS; ++i) { + if (supported_bats & BIT(i)) { + ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0); + if (ret) + break; + } + } + if (ret) { + dev_err(dev, "failed to register i2c mux channel %d\n", i + 1); + goto err_mux_register; + } + + psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc, + sizeof(struct power_supply_desc), + GFP_KERNEL); + if (!psy_desc) { + ret = -ENOMEM; + goto err_psy; + } + + psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s", + dev_name(&client->dev)); + if (!psy_desc->name) { + ret = -ENOMEM; + goto err_psy; + } + + psy_cfg.drv_data = data; + psy_cfg.of_node = dev->of_node; + data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg); + if (IS_ERR(data->psy)) { + ret = PTR_ERR(data->psy); + dev_err(dev, "failed to register power supply %s\n", + psy_desc->name); + goto err_psy; + } + + return 0; + +err_psy: +err_mux_register: + i2c_mux_del_adapters(data->muxc); + +err_mux_alloc: + return ret; +} + +static int sbsm_remove(struct i2c_client *client) +{ + struct sbsm_data *data = i2c_get_clientdata(client); + + i2c_mux_del_adapters(data->muxc); + return 0; +} + +static const struct i2c_device_id sbsm_ids[] = { + { "sbs-manager", 0 }, + { "ltc1760", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, sbsm_ids); + +#ifdef CONFIG_OF +static const struct of_device_id sbsm_dt_ids[] = { + { .compatible = "sbs,sbs-manager" }, + { .compatible = "lltc,ltc1760" }, + { } +}; +MODULE_DEVICE_TABLE(of, sbsm_dt_ids); +#endif + +static struct i2c_driver sbsm_driver = { + .driver = { + .name = "sbsm", + .of_match_table = of_match_ptr(sbsm_dt_ids), + }, + .probe = sbsm_probe, + .remove = sbsm_remove, + .id_table = sbsm_ids +}; +module_i2c_driver(sbsm_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Karl-Heinz Schneider "); +MODULE_DESCRIPTION("SBSM Smart Battery System Manager"); From a0b8839e2afc6613566ab723f8e3f0e698e9e007 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:09 +0800 Subject: [PATCH 09/13] power: supply: sbs-manager: Add alert callback and battery change notification This adds smb alert support via the smbus_alert driver to generate power_supply_changed notifications when either external power is removed / applied or a battery inserted / removed. Use the i2c alert callback to notify the attached battery driver that a change has occurred. Signed-off-by: Phil Reid Acked-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- drivers/power/supply/Kconfig | 3 +- drivers/power/supply/sbs-manager.c | 129 ++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index 1899b3ec6bec..fbca0ba7fc52 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -186,7 +186,8 @@ config CHARGER_SBS config MANAGER_SBS tristate "Smart Battery System Manager" - depends on I2C && I2C_MUX + depends on I2C && I2C_MUX && GPIOLIB + select I2C_SMBUS help Say Y here to include support for Smart Battery System Manager ICs. The driver reports online and charging status via sysfs. diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c index 47c09a178411..3e3ad09c5f11 100644 --- a/drivers/power/supply/sbs-manager.c +++ b/drivers/power/supply/sbs-manager.c @@ -16,10 +16,12 @@ * published by the Free Software Foundation. */ +#include #include #include #include #include +#include #define SBSM_MAX_BATS 4 #define SBSM_RETRY_CNT 3 @@ -43,7 +45,12 @@ struct sbsm_data { struct power_supply *psy; u8 cur_chan; /* currently selected channel */ + struct gpio_chip chip; bool is_ltc1760; /* special capabilities */ + + unsigned int supported_bats; + unsigned int last_state; + unsigned int last_state_cont; }; static enum power_supply_property sbsm_props[] = { @@ -186,6 +193,117 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) return ret; } +static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off) +{ + struct sbsm_data *data = gpiochip_get_data(gc); + int ret; + + ret = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE); + if (ret < 0) + return ret; + + return ret & BIT(off); +} + +/* + * This needs to be defined or the GPIO lib fails to register the pin. + * But the 'gpio' is always an input. + */ +static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned off) +{ + return 0; +} + +static int sbsm_do_alert(struct device *dev, void *d) +{ + struct i2c_client *client = i2c_verify_client(dev); + struct i2c_driver *driver; + + if (!client || client->addr != 0x0b) + return 0; + + device_lock(dev); + if (client->dev.driver) { + driver = to_i2c_driver(client->dev.driver); + if (driver->alert) + driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT, 0); + else + dev_warn(&client->dev, "no driver alert()!\n"); + } else + dev_dbg(&client->dev, "alert with no driver\n"); + device_unlock(dev); + + return -EBUSY; +} + +static void sbsm_alert(struct i2c_client *client, enum i2c_alert_protocol prot, + unsigned int d) +{ + struct sbsm_data *sbsm = i2c_get_clientdata(client); + + int ret, i, irq_bat = 0, state = 0; + + ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATE); + if (ret >= 0) { + irq_bat = ret ^ sbsm->last_state; + sbsm->last_state = ret; + state = ret; + } + + ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATECONT); + if ((ret >= 0) && + ((ret ^ sbsm->last_state_cont) & SBSM_BIT_AC_PRESENT)) { + irq_bat |= sbsm->supported_bats & state; + power_supply_changed(sbsm->psy); + } + sbsm->last_state_cont = ret; + + for (i = 0; i < SBSM_MAX_BATS; i++) { + if (irq_bat & BIT(i)) { + device_for_each_child(&sbsm->muxc->adapter[i]->dev, + NULL, sbsm_do_alert); + } + } +} + +static int sbsm_gpio_setup(struct sbsm_data *data) +{ + struct gpio_chip *gc = &data->chip; + struct i2c_client *client = data->client; + struct device *dev = &client->dev; + int ret; + + if (!device_property_present(dev, "gpio-controller")) + return 0; + + ret = sbsm_read_word(client, SBSM_CMD_BATSYSSTATE); + if (ret < 0) + return ret; + data->last_state = ret; + + ret = sbsm_read_word(client, SBSM_CMD_BATSYSSTATECONT); + if (ret < 0) + return ret; + data->last_state_cont = ret; + + gc->get = sbsm_gpio_get_value; + gc->direction_input = sbsm_gpio_direction_input; + gc->can_sleep = true; + gc->base = -1; + gc->ngpio = SBSM_MAX_BATS; + gc->label = client->name; + gc->parent = dev; + gc->owner = THIS_MODULE; + + ret = devm_gpiochip_add_data(dev, gc, data); + if (ret) { + dev_err(dev, "devm_gpiochip_add_data failed: %d\n", ret); + return ret; + } + + return ret; +} + static const struct power_supply_desc sbsm_default_psy_desc = { .type = POWER_SUPPLY_TYPE_MAINS, .properties = sbsm_props, @@ -203,7 +321,7 @@ static int sbsm_probe(struct i2c_client *client, struct device *dev = &client->dev; struct power_supply_desc *psy_desc; struct power_supply_config psy_cfg = {}; - int ret = 0, i, supported_bats; + int ret = 0, i; /* Device listens only at address 0x0a */ if (client->addr != 0x0a) @@ -224,8 +342,7 @@ static int sbsm_probe(struct i2c_client *client, ret = sbsm_read_word(client, SBSM_CMD_BATSYSINFO); if (ret < 0) return ret; - supported_bats = ret & SBSM_MASK_BAT_SUPPORTED; - + data->supported_bats = ret & SBSM_MASK_BAT_SUPPORTED; data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0, I2C_MUX_LOCKED, &sbsm_select, NULL); if (!data->muxc) { @@ -237,7 +354,7 @@ static int sbsm_probe(struct i2c_client *client, /* register muxed i2c channels. One for each supported battery */ for (i = 0; i < SBSM_MAX_BATS; ++i) { - if (supported_bats & BIT(i)) { + if (data->supported_bats & BIT(i)) { ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0); if (ret) break; @@ -262,6 +379,9 @@ static int sbsm_probe(struct i2c_client *client, ret = -ENOMEM; goto err_psy; } + ret = sbsm_gpio_setup(data); + if (ret < 0) + goto err_psy; psy_cfg.drv_data = data; psy_cfg.of_node = dev->of_node; @@ -314,6 +434,7 @@ static struct i2c_driver sbsm_driver = { }, .probe = sbsm_probe, .remove = sbsm_remove, + .alert = sbsm_alert, .id_table = sbsm_ids }; module_i2c_driver(sbsm_driver); From 1cf855535b034579ac4d39f49da853594ce968dc Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 24 Aug 2017 17:31:10 +0800 Subject: [PATCH 10/13] power: supply: sbs-battery: move gpio present detect to sbs_get_property Currently when a gpio is defined for battery presence it is only used in the sbs_get_battery_presence_and_health function for 2 properties. All other properties currently try to read data form the battery before returning an error if not present. We should know in advance that no data is going to returned. As the driver tries multiple times to access a property, this prevents a lot of smbus accesses, which had a significant effect on device boot-up. As when the device is registered lots of property accesses are attempted during boot. If no gpio is used for presence detection no change in behaviour should occur. Signed-off-by: Phil Reid Reviewed-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- drivers/power/supply/sbs-battery.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index b19a73176910..8dd4bd70c561 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -321,16 +321,6 @@ static int sbs_get_battery_presence_and_health( union power_supply_propval *val) { s32 ret; - struct sbs_info *chip = i2c_get_clientdata(client); - - if (psp == POWER_SUPPLY_PROP_PRESENT && chip->gpio_detect) { - ret = gpiod_get_value_cansleep(chip->gpio_detect); - if (ret < 0) - return ret; - val->intval = ret; - chip->is_present = val->intval; - return ret; - } /* * Write to ManufacturerAccess with ManufacturerAccess command @@ -598,6 +588,19 @@ static int sbs_get_property(struct power_supply *psy, struct sbs_info *chip = power_supply_get_drvdata(psy); struct i2c_client *client = chip->client; + if (chip->gpio_detect) { + ret = gpiod_get_value_cansleep(chip->gpio_detect); + if (ret < 0) + return ret; + if (psp == POWER_SUPPLY_PROP_PRESENT) { + val->intval = ret; + chip->is_present = val->intval; + return 0; + } + if (ret == 0) + return -ENODATA; + } + switch (psp) { case POWER_SUPPLY_PROP_PRESENT: case POWER_SUPPLY_PROP_HEALTH: From 9410b7d71032cc76c4211cbf8f76bad322f0c334 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Sun, 29 Oct 2017 00:42:10 +0200 Subject: [PATCH 11/13] power: supply: sbs-battery: remove superfluous variable init Those variables are immediately assigned a value afterwards. Reviewed-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- drivers/power/supply/sbs-battery.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index 8dd4bd70c561..fb61da370947 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -177,10 +177,8 @@ static bool force_load; static int sbs_read_word_data(struct i2c_client *client, u8 address) { struct sbs_info *chip = i2c_get_clientdata(client); + int retries = chip->i2c_retry_count; s32 ret = 0; - int retries = 1; - - retries = chip->i2c_retry_count; while (retries > 0) { ret = i2c_smbus_read_word_data(client, address); @@ -204,7 +202,7 @@ static int sbs_read_string_data(struct i2c_client *client, u8 address, { struct sbs_info *chip = i2c_get_clientdata(client); s32 ret = 0, block_length = 0; - int retries_length = 1, retries_block = 1; + int retries_length, retries_block; u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1]; retries_length = chip->i2c_retry_count; @@ -269,10 +267,8 @@ static int sbs_write_word_data(struct i2c_client *client, u8 address, u16 value) { struct sbs_info *chip = i2c_get_clientdata(client); + int retries = chip->i2c_retry_count; s32 ret = 0; - int retries = 1; - - retries = chip->i2c_retry_count; while (retries > 0) { ret = i2c_smbus_write_word_data(client, address, value); From 5e9bee5267fe626df39ca04d6c30e135a9e8b157 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Sun, 29 Oct 2017 00:45:59 +0200 Subject: [PATCH 12/13] power: supply: sbs-battery: remove unchecked return var Since the return value is not checked anyhow, we don't need to store it. Reviewed-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- drivers/power/supply/sbs-battery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index fb61da370947..83d7b4115857 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -556,7 +556,7 @@ static int sbs_get_battery_serial_number(struct i2c_client *client, if (ret < 0) return ret; - ret = sprintf(sbs_serial, "%04x", ret); + sprintf(sbs_serial, "%04x", ret); val->strval = sbs_serial; return 0; From 4cf419a2b4c2e5bb0583e32e5fe079995c987c0f Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Sun, 29 Oct 2017 00:49:14 +0200 Subject: [PATCH 13/13] power: supply: sbs-message: fix some code style issues Use 'unsigned int' and curly braces for 'else'. Reviewed-by: Sebastian Reichel Signed-off-by: Wolfram Sang --- drivers/power/supply/sbs-manager.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c index 3e3ad09c5f11..ccb4217b9638 100644 --- a/drivers/power/supply/sbs-manager.c +++ b/drivers/power/supply/sbs-manager.c @@ -193,7 +193,7 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) return ret; } -static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off) +static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned int off) { struct sbsm_data *data = gpiochip_get_data(gc); int ret; @@ -209,7 +209,7 @@ static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off) * This needs to be defined or the GPIO lib fails to register the pin. * But the 'gpio' is always an input. */ -static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned off) +static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned int off) { return 0; } @@ -229,8 +229,9 @@ static int sbsm_do_alert(struct device *dev, void *d) driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT, 0); else dev_warn(&client->dev, "no driver alert()!\n"); - } else + } else { dev_dbg(&client->dev, "alert with no driver\n"); + } device_unlock(dev); return -EBUSY;