Hi Uwe, 2017-01-11 16:42 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>: > Hello Cedric, > > On Wed, Jan 11, 2017 at 03:20:41PM +0100, M'boumba Cedric Madianga wrote: >> > >> >> + */ >> >> + reg = i2c_dev->base + STM32F4_I2C_CR1; >> >> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK); >> >> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS); >> > >> > You could get rid of this, when caching the value of CR1. Would save two >> > register reads here. This doesn't work for all registers, but it should >> > be possible to apply for most of them, maybe enough to get rid of the >> > clr_bits and set_bits function. >> >> I agree at many places I could save registers read by not using >> clr_bits and set_bits function when the registers in question has been >> already read. >> But it is not enough to get rid of the clr_bits and set_bits function. >> For example when calling stm32f4_i2c_terminate_xfer(), the CR1 >> register is never read before so set_bits function is useful. > > I didn't double check the manual, but I would expect that CR1 isn't > modified by hardware. So you can cache the result in the driver data > structure and do the necessary modifications with that one. CR1 is modified by hardware to clear some bits set by software during the communication. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ | Best regards, Cedric -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
