Hi Björn,
thanks for this patch!
On Tue, Aug 13, 2019 at 07:55:08AM +0200, Björn Ardö wrote:
> Add a 32 and a 64 kbit memory. These needs 16 bit address
> so added support for that as well.
>
May I ask which Linux I2C driver you use to act as a slave?
> 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.
>
> 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?
> 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.
> 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?
Kind regards,
Wolfram
Attachment:
signature.asc
Description: PGP signature
