Hi Sven, On Fri, Sep 24, 2021 at 04:58:52PM +0200, Sven Peter wrote: > > Couldn't you just use the compatible property and local variable here? > > > > irq_handler_t irq_handler = tps6598x_interrupt; > > struct device_node *np = client->dev.of_node; > > > > if (np && of_device_is_compatible(np, "apple,cd321x")) { > > /* CD321X chips have all interrupts masked initially */ > > ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, > > APPLE_CD_REG_INT_POWER_STATUS_UPDATE | > > APPLE_CD_REG_INT_DATA_STATUS_UPDATE | > > APPLE_CD_REG_INT_PLUG_EVENT); > > if (ret) > > return ret; > > > > irq_handler = cd321x_interrupt; > > } > > > > ret = devm_request_threaded_irq(&client->dev, client->irq, NULL, > > irq_handler, > > IRQF_SHARED | IRQF_ONESHOT, > > dev_name(&client->dev), tps); > > > > I did not go over the whole series yet, so I may have missed > > something. > > Sure, that will work and get rid of the slightly awkward and redundant interrupt/enum > combination. I'll wait for your comments for the rest of the series and do that for v3 then :) The rest of the series look OK to me. thanks, -- heikki