Hi,
On Wed, Nov 27, 2019 at 10:06:47PM -0300, Matheus Castello wrote:
> [...]
> > > @@ -256,14 +303,26 @@ static int max17040_probe(struct i2c_client *client,
> > >
> > > /* check interrupt */
> > > if (client->irq) {
> > > - int ret;
> > > -
> > > - ret = max17040_enable_alert_irq(chip);
> > > -
> > > - if (ret) {
> > > - client->irq = 0;
> > > + if (of_device_is_compatible(client->dev.of_node,
> > > + "maxim,max77836-battery")) {
> > > + ret = max17040_set_low_soc_alert(client,
> > > + chip->low_soc_alert);
> > > + if (ret) {
> > > + dev_err(&client->dev,
> > > + "Failed to set low SOC alert: err %d\n",
> > > + ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = max17040_enable_alert_irq(chip);
> > > + if (ret) {
> > > + client->irq = 0;
> > > + dev_warn(&client->dev,
> > > + "Failed to get IRQ err %d\n", ret);
> > > + }
> > > + } else {
> > > dev_warn(&client->dev,
> > > - "Failed to get IRQ err %d\n", ret);
> > > + "Device not compatible for IRQ");
> >
> > Something is odd here. Either this should be part of the first
> > patch ("max17040: Add IRQ handler for low SOC alert"), or both
> > device types support the IRQ and this check should be removed.
>
> The first patch add the IRQ without the configuration of the low SoC alert,
> using the default state of charge level. This patch is working with
> registers to config the low state of charge level, so it was proposed to
> just try to write registers in the models compatible with that
> (maxim,max77836-battery).
>
> Maybe join the first patch to this one, and let DT binding be the first
> patch on the series so we can already test compatible here, let me know what
> you think about it.
Assuming the !max77836 do not have any interrupt support, you can
just add the OF check in the first patch in "if (client->irq)", so
that it reads
if (client->irq && of_device_is_compatible(...)) {
...
}
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
