Hi,
On 08-22 09:40, Corey Minyard wrote:
> > > +++ b/drivers/char/ipmi/ipmi_ssif.c
> > > @@ -1368,8 +1368,20 @@ static int ssif_detect(struct i2c_client *client,
> > > struct i2c_board_info *info)
> > > rv = do_cmd(client, 2, msg, &len, resp);
> > > if (rv)
> > > rv = -ENODEV;
> >
> > What is my worry is that in case of SMBus errors, device is there but
> > for some reason it got stuck/crashed or whatever, so will get out of
> > detect function from here and with ENODEV return code probe function
> > will be called for no reason.
>
> That's not how the i2c code works. See my next comment.
>
> >
> > > - else
> > > + else {
> > > + if (len < 3) {
> > > + rv = -ENODEV;
> >
> > No point to call probe(), right?
>
> Originally (before I add the call from ssif_probe()), this is not involved in
> the probe() call. Instead, the detect function is involved in calling a
> table of addresses in driver->address_list. So in this case this
> function is never called at all from the i2c code, since there is no
> address list.
I see, thank you for explanation.
> > >
> > > + /*
> > > + * Send a get device id command and validate its response to
> > > + * make sure a valid BMC is there.
> > > + */
> > > + rv = ssif_detect(client, NULL);
> > > + if (rv) {
> > > + dev_err(&client->dev, "Not present\n");
> > > + goto out;
> > > + }
> > > +
> >
> > The point is that even after this point IPMI device can start failing
> > to properly communicate with the OS, real SMBus errors, like EREMOTEIO
> > in my case, but unfortunately code bellow do not handle this very well,
> > I think.
>
> It is possible that the BMC gets rebooted or something between the call
> to ssif_detect() and the code below, but the probability is really low.
> If it answers a detect, the rest of the things should work.
>
I have my daubs, but patch proposed will fix the issue that I see.
Thank you,
Ivan
_______________________________________________
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
