Messages in this thread | | | Date | Mon, 20 Nov 2023 10:26:29 +0100 | Subject | Re: [PATCH 6/7] w1: ds2433: add support for ds28ec20 eeprom | From | Krzysztof Kozlowski <> |
| |
On 17/11/2023 20:29, marc.ferland@gmail.com wrote: > From: Marc Ferland <marc.ferland@sonatest.com> > > The ds28ec20 eeprom is (almost) backward compatible with the > ds2433. The only major differences are: > > - the eeprom size is now 2560 bytes instead of 512; > - the number of pages is now 80 (same page size as the ds2433: 256 bits); > - the programming time has increased from 5ms to 10ms; > > This patch adds support for the ds28ec20 to the ds2433 driver. From > the datasheet: The DS28EC20 provides a high degree of backward > compatibility with the DS2433. Besides the different family codes, the > only protocol change that is required on an existing DS2433 > implementation is a lengthening of the programming duration (tPROG) > from 5ms to 10ms. > > Tests: > > dmesg now returns: > > w1_master_driver w1_bus_master1: Attaching one wire slave 43.000000478756 crc e0 > > instead of: > > w1_master_driver w1_bus_master1: Attaching one wire slave 43.000000478756 crc e0 > w1_master_driver w1_bus_master1: Family 43 for 43.000000478756.e0 is not registered. > > Test script: > > #!/bin/sh > > EEPROM=/sys/bus/w1/devices/43-000000478756/eeprom > BINFILE1=/home/root/file1.bin > BINFILE2=/home/root/file2.bin > > for BS in 1 2 3 4 8 16 32 64 128 256 512 1024 2560; do > dd if=/dev/random of=${BINFILE1} bs=${BS} count=1 status=none > dd if=${BINFILE1} of=${EEPROM} status=none > dd if=${EEPROM} of=${BINFILE2} bs=${BS} count=1 status=none > if ! cmp --silent ${BINFILE1} ${BINFILE2}; then > echo file1 > hexdump ${BINFILE1} > echo file2 > hexdump ${BINFILE2} > echo FAIL > exit 1 > fi > echo "${BS} OK!" > done > > Test results (CONFIG_W1_SLAVE_DS2433_CRC is not set): > > $ cat /proc/config.gz | gunzip | grep CONFIG_W1_SLAVE_DS2433 > CONFIG_W1_SLAVE_DS2433=m > # CONFIG_W1_SLAVE_DS2433_CRC is not set > > # ./test.sh > 1 OK! > 2 OK! > 3 OK! > 4 OK! > 8 OK! > 16 OK! > 32 OK! > 64 OK! > 128 OK! > 256 OK! > 512 OK! > 1024 OK! > 2560 OK! > > Test results (CONFIG_W1_SLAVE_DS2433_CRC=y): > > $ cat /proc/config.gz | gunzip | grep CONFIG_W1_SLAVE_DS2433 > CONFIG_W1_SLAVE_DS2433=m > CONFIG_W1_SLAVE_DS2433_CRC=y > > # create a 32 bytes block with a crc, i.e.: > 00000000 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 |123456789:;<=>?@| > 00000010 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e ba 63 |ABCDEFGHIJKLMN.c| > > # fill all 80 blocks > $ dd if=test.bin of=/sys/bus/w1/devices/43-000000478756/eeprom bs=32 count=80 > > # read back all blocks, i.e.: > $ hexdump -C /sys/bus/w1/devices/43-000000478756/eeprom > 00000000 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 |123456789:;<=>?@| > 00000010 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e ba 63 |ABCDEFGHIJKLMN.c| > 00000020 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 |123456789:;<=>?@| > 00000030 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e ba 63 |ABCDEFGHIJKLMN.c| > ... > 000009e0 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 |123456789:;<=>?@| > 000009f0 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e ba 63 |ABCDEFGHIJKLMN.c| > 00000a00 > > Signed-off-by: Marc Ferland <marc.ferland@sonatest.com> > --- > drivers/w1/slaves/w1_ds2433.c | 84 +++++++++++++++++++++++++++++++---- > 1 file changed, 75 insertions(+), 9 deletions(-) > > diff --git a/drivers/w1/slaves/w1_ds2433.c b/drivers/w1/slaves/w1_ds2433.c > index 04c3eee9e5d7..69bdf3dba573 100644 > --- a/drivers/w1/slaves/w1_ds2433.c > +++ b/drivers/w1/slaves/w1_ds2433.c > @@ -1,8 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * w1_ds2433.c - w1 family 23 (DS2433) driver > + * w1_ds2433.c - w1 family 23 (DS2433) & 43 (DS28EC20) eeprom driver > * > * Copyright (c) 2005 Ben Gardner <bgardner@wabtec.com> > + * Copyright (c) 2023 Marc Ferland <marc.ferland@sonatest.com> > */ > > #include <linux/kernel.h> > @@ -23,6 +24,7 @@ > #include <linux/w1.h> > > #define W1_F23_EEPROM_DS2433 0x23 > +#define W1_F43_EEPROM_DS28EC20 0x43 > > #define W1_PAGE_SIZE 32 > #define W1_PAGE_BITS 5 > @@ -45,10 +47,16 @@ static const struct ds2433_config config_f23 = { > .tprog = 5, > }; > > +static const struct ds2433_config config_f43 = { > + .eeprom_size = 2560, > + .page_count = 80, > + .tprog = 10, > +}; > + > struct w1_data { > #ifdef CONFIG_W1_SLAVE_DS2433_CRC > - u8 *memory; > - u32 validcrc; > + u8 *memory; > + unsigned long *validcrc;
Why do you change it? This is actually candidate for its own patch with its own justification (not "groundwork" but a reason why such code is better).
Best regards, Krzysztof
| |