Hi Wolfram,
On 8/31/19 3:22 PM, Wolfram Sang wrote:
May I ask which Linux I2C driver you use to act as a slave?
This is the i2c-controller in the ARTPEC-6 system. The driver itself is
not yet upstreamed.
Signed-off-by: Björn Ardö <bjorn.ardo@xxxxxxxx>
---
drivers/i2c/i2c-slave-eeprom.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
index be65d38..35bff28 100644
--- a/drivers/i2c/i2c-slave-eeprom.c
+++ b/drivers/i2c/i2c-slave-eeprom.c
@@ -18,15 +18,22 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/sysfs.h>
+#include <linux/bitfield.h>
Please keep it sorted.
Will fix!
struct eeprom_data {
struct bin_attribute bin;
- bool first_write;
+ int write_nbr;
What does 'nbr' stand for? My gut feeling is that this variable could
have a more speaking name. Also, at least unsigned, maybe just u8?
My line of though here was that the previous code used a flag called
"first_write" to keep track of which part of the write is the address.
With 16 bit addresses we need to keep track of both the first and the
second write, thus a variable that keeps track of which write it is
(first, second, third, etc..), thus the name "write_nbr". What would be
a better name, "write_cnt", "initial_write_cnt"?
u8 should be sufficient, will fix!
spinlock_t buffer_lock;
- u8 buffer_idx;
+ u16 buffer_idx;
+ u16 address_mask;
+ u8 address_bytes;
num_address_bytes makes the code easier to read IMO.
will fix!
u8 buffer[];
};
Code itself looks good to me. I'd like to test it, though, hopefully
until Tuesday. What kind of tests did you perform?
The use case we have is to have this code running on one system and load the virtual eeprom with a static file. Then another system will read this data over i2c. This scenario is the one we tested the most. During development I also tested to connect this device to another i2c port on the same system and cat files to the entries in /sys in both directions and verify that they are the same. I have not run any extensive test-suit that tested every possible combinations of read and write commands.
Best Regards,
Björn
