On 29/07/2023 18:08, Svyatoslav Ryhel wrote: > From: Michał Mirosław <mirq-linux@xxxxxxxxxxxx> > > Implement driver for hot-plugged I2C busses, where some devices on > a bus are hot-pluggable and their presence is indicated by GPIO line. > > This feature is mainly used by the ASUS Transformers family. The > Transformers have a connector that's used for USB, charging or for > attaching a dock-keyboard (which also has a battery and a touchpad). > This connector probably (can't be verified since no datasheets or > special equipment is available) has an I2C bus lines and a "detect" > line (pulled low on the dock side) among the pins. I guess there > is either no additional chip or a transparent bridge/buffer chip, > but nothing that could be controlled by software. For DT this setup > could be modelled like an I2C gate or 2-port mux with enable joining > two I2C buses (one "closer" to the CPU as a parent). > > Co-developed-by: Ion Agorria <ion@xxxxxxxxxxx> > Signed-off-by: Ion Agorria <ion@xxxxxxxxxxx> > Signed-off-by: Michał Mirosław <mirq-linux@xxxxxxxxxxxx> > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx> ... > + ret = devm_add_action_or_reset(&pdev->dev, devm_i2c_put_adapter, > + parent); > + if (ret) > + return ret; > + > + priv->gpio = devm_gpiod_get(&pdev->dev, "detect", GPIOD_IN); > + if (IS_ERR(priv->gpio)) > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->gpio), > + "failed to get detect GPIO\n"); > + > + is_i2c = parent->algo->master_xfer; > + is_smbus = parent->algo->smbus_xfer; > + > + snprintf(priv->adap.name, sizeof(priv->adap.name), > + "i2c-hotplug (master i2c-%d)", i2c_adapter_id(parent)); > + priv->adap.owner = THIS_MODULE; > + priv->adap.algo = i2c_hotplug_algo[is_i2c][is_smbus]; > + priv->adap.algo_data = NULL; > + priv->adap.lock_ops = &i2c_hotplug_lock_ops; > + priv->adap.class = parent->class; > + priv->adap.retries = parent->retries; > + priv->adap.timeout = parent->timeout; > + priv->adap.quirks = parent->quirks; > + if (parent->bus_recovery_info) > + priv->adap.bus_recovery_info = &i2c_hotplug_recovery_info; > + > + if (!priv->adap.algo) > + return -EINVAL; > + > + priv->irq = platform_get_irq(pdev, 0); > + if (priv->irq < 0) > + return dev_err_probe(&pdev->dev, priv->irq, > + "failed to get IRQ %d\n", priv->irq); > + > + ret = devm_request_threaded_irq(&pdev->dev, priv->irq, NULL, > + i2c_hotplug_interrupt, > + IRQF_ONESHOT | IRQF_SHARED, Shared IRQ with devm is a recipe for disaster. Are you sure this is a shared one? You have a remove() function which also points that it is not safe. You can: 1. investigate to be sure it is 100% safe (please document why do you think it is safe) 2. drop devm 3. drop shared flag. Best regards, Krzysztof
