On 14.05.2021 15:51:01, Torin Cooper-Bennun wrote: > On Fri, May 14, 2021 at 04:10:12PM +0200, Marc Kleine-Budde wrote: > > For new code, please don't wrap the regmap_*() functions so that the > > error values are ignored. I know, it's a bit annoying to always do the > > "if (err) return err" dance. > > Will do, thanks. > > > As this is the interrupt handler there's not much we can do in case of > > an error. In the mcp251xfd driver I print an error message and shut down > > the interface. You should at least print an error message at the end of > > the handle_interrupts() function. > > I assume you mean the handling of errors out of aforementioned > regmap_*() functions specifically? ACK, I'm talking about the return value of the regmap_() functions. > I agree we should shut down the interface - I've previously endured > serious debugging pain due to SPI errors going unrecognised and > causing havoc. You can eventually do the same, return negative error value from the tcan4x5x handler and the main IRQ handler will shut down the chip. A return value >= 0 would be the IRQ_*. Some background information: You have to keep in mind, the regmap_() functions might fail due to a non permanent problem, e.g. resource shortage (out-of-memory, etc...). I decided it's too complicated to properly recover from those, especially if the driver touches some FIFO pointer. The mcp251xfd chip offers CRC check summing during read. If the driver detect a CRC read error (that stays even after some retries), I let the regmap_read() function to return an error that is then passed down the call stack. And then chip will be shut down. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
