[coreboot-gerrit] Change in coreboot[master]: drivers/i2c: Add new driver for RTC type RX6110 SA

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/19625 )

Change subject: drivers/i2c: Add new driver for RTC type RX6110 SA
......................................................................


Patch Set 2:

(11 comments)

Thanks for the review!

https://review.coreboot.org/#/c/19625/1//COMMIT_MSG
Commit Message:

PS1, Line 9: th
> the
Done


PS1, Line 11: us
> in the
Done


https://review.coreboot.org/#/c/19625/1/src/drivers/i2c/rx6110sa/chip.h
File src/drivers/i2c/rx6110sa/chip.h:

PS1, Line 19: /* The day (of the week) is indicated by 7 bits, bit 0 to bit
> Can you please add in the comment, that the weekday has to have the value 0
Done


https://review.coreboot.org/#/c/19625/1/src/drivers/i2c/rx6110sa/rx6110sa.c
File src/drivers/i2c/rx6110sa/rx6110sa.c:

PS1, Line 16: #include <device/i2c.h>
> The whole driver references I2C transfaers.
is not used - removed


PS1, Line 18: #include <version.h>
> You don't have to use soc/i2c.h as your driver completely relies on device/
changed with #include <device/device.h>


PS1, Line 81: data loss, s
> "power loss event"?
Done


PS1, Line 85: 
> detected,
Done


PS1, Line 88: 
> detected,
Done


PS1, Line 95: b(I2C_BUS_NUM, I
> In the other comments you have used lower case for bit descriptions. Want t
Done


PS1, Line 122: 
> Maybe better call it "RTC" here as the complete RTC is initialized now?
Done


PS1, Line 123: i2c_writeb(I2C_BUS_NUM, I2C_DEV_NUM, HOUR_REG, 1);
             : 	i2c_writeb(I2C_BUS_NUM, I2C_DEV_NUM, MINUTE_REG, 0);
             : 	i2c_writeb(I2C_BUS_NUM, I2C_DEV_NUM, SECOND_REG, 0);
> In Line 107 you have written STOP_BIT to control register.
Done


-- 
To view, visit https://review.coreboot.org/19625
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1290a10c2d5ad76a317c99c8b92a013309a605d6
Gerrit-PatchSet: 2
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Mario Scheithauer <mario.scheithauer at siemens.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer at siemens.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh at siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes