Messages in this thread | | | Date | Tue, 5 May 2020 16:49:00 +0200 | From | Greg KH <> | Subject | Re: [PATCH v4 2/9] w1_therm: fix reset_select_slave at beginning of search process |
| |
On Thu, Apr 30, 2020 at 12:59:43AM +0200, Akira Shimahara wrote: > Fix reset_select_slave issue during devices discovery by the master on > bus. The w1_reset_select_slave() from w1_io.c, which was previously used, > assume that if the slave count is 1 there is only one slave attached on > the bus. This is not always true. For example when discovering devices, > when the first device is discover by the bus master, its slave count is > 1, but some other slaves may be on the bus. > > In that case instead of adressing command to the attached slave the > master throw a SKIP ROM command so that all slaves attached on the bus > will answer simultenaously causing data collision. > > A dedicated reset_select_slave() function is implemented here, > it always perform an adressing to each slave using the MATCH ROM > command. > > Signed-off-by: Akira Shimahara <akira215corp@gmail.com> > --- > drivers/w1/slaves/w1_therm.c | 29 ++++++++++++++++++++++------- > drivers/w1/slaves/w1_therm.h | 13 +++++++++++++ > 2 files changed, 35 insertions(+), 7 deletions(-) > > diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c > index f027360..6245950 100644 > --- a/drivers/w1/slaves/w1_therm.c > +++ b/drivers/w1/slaves/w1_therm.c > @@ -235,7 +235,7 @@ static inline int w1_DS18B20_precision(struct device *device, int val) > while (max_trying--) { > crc = 0; > > - if (!w1_reset_select_slave(sl)) { > + if (!reset_select_slave(sl)) { > int count = 0; > > /* read values to only alter precision bits */ > @@ -248,7 +248,7 @@ static inline int w1_DS18B20_precision(struct device *device, int val) > if (rom[8] == crc) { > rom[4] = (rom[4] & ~mask) | (precision_bits & mask); > > - if (!w1_reset_select_slave(sl)) { > + if (!reset_select_slave(sl)) { > w1_write_8(dev, W1_WRITE_SCRATCHPAD); > w1_write_8(dev, rom[2]); > w1_write_8(dev, rom[3]); > @@ -319,6 +319,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl) > > /*------------------------Hardware Functions--------------------------*/ > > +/* Safe version of reser_select_slave - avoid using the one in w_io.c */ > +static int reset_select_slave(struct w1_slave *sl) > +{ > + u8 match[9] = { W1_MATCH_ROM, }; > + u64 rn = le64_to_cpu(*((u64 *)&sl->reg_num)); > + > + if (w1_reset_bus(sl->master)) > + return -ENODEV; > + > + memcpy(&match[1], &rn, 8); > + w1_write_block(sl->master, match, 9); > + > + return 0; > +}
If you put this higher up in the .c file, no function definition is needed in the .h file at all, right?
thanks,
greg k-h
| |