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 <akira215c...@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
