Hi Russell, >> Searching the driver for "air_buckpbus_reg_read", there are four >> instances where you read-modify-write, and only one instance which is >> just a read. >> >> I wonder whether it would make more sense to structure the accessors as >> >> __air_buckpbus_set_address(phydev, addr) >> __air_buckpbus_read(phydev, *data) >> __air_buckpbus_write(phydev, data) >> >> which would make implementing reg_read, reg_write and reg_modify() >> easier, and the addition of reg_modify() means that (a) there are less >> bus cycles (through having to set the address twice) and (b) ensures >> that the read-modify-write can be done atomically on the bus. This >> assumes that reading data doesn't auto-increment the address (which >> you would need to check.) > > I will see if I can change the code to: > > __air_buckpbus_set_address(phydev, addr) > __air_buckpbus_read(phydev, *data) > __air_buckpbus_write(phydev, data) > air_buckpbus_reg_read() > air_buckpbus_reg_write() > air_buckpbus_reg_modify() > > While not changing (except if (saved_page >= 0)): > > __air_write_buf() > air_write_buf() > I just remember, the address register for write, is a different register from the address register for read. So we always have to write to address registers twice. I can still implement an atomic air_buckpbus_reg_modify() though. Best regards, Eric Woudstra
