[PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
found. Rebooting on the other hand appears to fix the issue. I found
that if I modified the timing slightly (but still within spec) the
w1 identification on cold boot became far more reliable.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/w1/w1-gpio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
index 9346f810ce..5565de2a92 100644
--- a/drivers/w1/w1-gpio.c
+++ b/drivers/w1/w1-gpio.c
@@ -22,8 +22,8 @@
 #define W1_TIMING_E	9
 #define W1_TIMING_F	55
 #define W1_TIMING_G	0
-#define W1_TIMING_H	480
-#define W1_TIMING_I	70
+#define W1_TIMING_H	600
+#define W1_TIMING_I	100
 #define W1_TIMING_J	410
 
 struct w1_gpio_pdata {
-- 
2.30.2

Re: [PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Eugen.Hristev]

On 11/8/21 5:07 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
> found. Rebooting on the other hand appears to fix the issue. I found
> that if I modified the timing slightly (but still within spec) the
> w1 identification on cold boot became far more reliable.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>   drivers/w1/w1-gpio.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
> index 9346f810ce..5565de2a92 100644
> --- a/drivers/w1/w1-gpio.c
> +++ b/drivers/w1/w1-gpio.c
> @@ -22,8 +22,8 @@
>   #define W1_TIMING_E    9
>   #define W1_TIMING_F    55
>   #define W1_TIMING_G    0
> -#define W1_TIMING_H    480
> -#define W1_TIMING_I    70
> +#define W1_TIMING_H    600
> +#define W1_TIMING_I    100
>   #define W1_TIMING_J    410
> 
>   struct w1_gpio_pdata {
> --
> 2.30.2
> 

Hi,

Thanks for your patch,

I will test this on my boards at some point, but I can't promise exactly 
when.
If there are any other users of this protocol, I think they should test 
it as well.

Eugen

Re: [PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Eugen.Hristev]

On 11/8/21 5:07 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
> found. Rebooting on the other hand appears to fix the issue. I found
> that if I modified the timing slightly (but still within spec) the
> w1 identification on cold boot became far more reliable.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>   drivers/w1/w1-gpio.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
> index 9346f810ce..5565de2a92 100644
> --- a/drivers/w1/w1-gpio.c
> +++ b/drivers/w1/w1-gpio.c
> @@ -22,8 +22,8 @@
>   #define W1_TIMING_E    9
>   #define W1_TIMING_F    55
>   #define W1_TIMING_G    0
> -#define W1_TIMING_H    480
> -#define W1_TIMING_I    70
> +#define W1_TIMING_H    600
> +#define W1_TIMING_I    100
>   #define W1_TIMING_J    410
> 
>   struct w1_gpio_pdata {
> --
> 2.30.2
> 


Hi Chris,

I tested your patch on my board sama5d2_xplained, and it works.
Thus, you can add my
Tested-by: Eugen Hristev <eugen.hristev@microchip.com>

However, I disagree with the changes you did in timings. What I found 
was that timing 'H' could go up to 640 , but timing 'I' to a maximum of 
75 or so. [1]

I am thinking maybe you could also check your udelays with a scope on 
the 1wire line ? Because your problem might be in fact in some other 
part , like udelays not properly aligned/synchronized/accurate at cold 
boot time, depending on the source of clock you are using.

Eugen


[1] 
https://www.maximintegrated.com/content/dam/files/design/tools/tech-docs/126/AN126-timing-calculation.zip

Re: [PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Chris Morgan]

On Mon, Nov 22, 2021 at 11:16:22AM +0000, Eugen.Hristev@microchip.com wrote:
> On 11/8/21 5:07 PM, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
> > found. Rebooting on the other hand appears to fix the issue. I found
> > that if I modified the timing slightly (but still within spec) the
> > w1 identification on cold boot became far more reliable.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >   drivers/w1/w1-gpio.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
> > index 9346f810ce..5565de2a92 100644
> > --- a/drivers/w1/w1-gpio.c
> > +++ b/drivers/w1/w1-gpio.c
> > @@ -22,8 +22,8 @@
> >   #define W1_TIMING_E    9
> >   #define W1_TIMING_F    55
> >   #define W1_TIMING_G    0
> > -#define W1_TIMING_H    480
> > -#define W1_TIMING_I    70
> > +#define W1_TIMING_H    600
> > +#define W1_TIMING_I    100
> >   #define W1_TIMING_J    410
> > 
> >   struct w1_gpio_pdata {
> > --
> > 2.30.2
> > 
> 
> 
> Hi Chris,
> 
> I tested your patch on my board sama5d2_xplained, and it works.
> Thus, you can add my
> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
> 
> However, I disagree with the changes you did in timings. What I found 
> was that timing 'H' could go up to 640 , but timing 'I' to a maximum of 
> 75 or so. [1]
> 
> I am thinking maybe you could also check your udelays with a scope on 
> the 1wire line ? Because your problem might be in fact in some other 
> part , like udelays not properly aligned/synchronized/accurate at cold 
> boot time, depending on the source of clock you are using.

I lack a scope, but will extensively test 640 and 75 as the new timings.
Would that be acceptable?

Thank you.

> 
> Eugen
> 
> 
> [1] 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.maximintegrated.com%2Fcontent%2Fdam%2Ffiles%2Fdesign%2Ftools%2Ftech-docs%2F126%2FAN126-timing-calculation.zip&amp;data=04%7C01%7C%7C6787b70951c4494fd76808d9ada98a17%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637731765952771191%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V81HrXrKI8d45WTrvWrc6ydJy2rdCX5%2FL8eYjvxcwCs%3D&amp;reserved=0

Re: [PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Eugen.Hristev]

On 11/30/21 5:46 PM, Chris Morgan wrote:
> On Mon, Nov 22, 2021 at 11:16:22AM +0000, Eugen.Hristev@microchip.com wrote:
>> On 11/8/21 5:07 PM, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
>>> found. Rebooting on the other hand appears to fix the issue. I found
>>> that if I modified the timing slightly (but still within spec) the
>>> w1 identification on cold boot became far more reliable.
>>>
>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>> ---
>>>    drivers/w1/w1-gpio.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
>>> index 9346f810ce..5565de2a92 100644
>>> --- a/drivers/w1/w1-gpio.c
>>> +++ b/drivers/w1/w1-gpio.c
>>> @@ -22,8 +22,8 @@
>>>    #define W1_TIMING_E    9
>>>    #define W1_TIMING_F    55
>>>    #define W1_TIMING_G    0
>>> -#define W1_TIMING_H    480
>>> -#define W1_TIMING_I    70
>>> +#define W1_TIMING_H    600
>>> +#define W1_TIMING_I    100
>>>    #define W1_TIMING_J    410
>>>
>>>    struct w1_gpio_pdata {
>>> --
>>> 2.30.2
>>>
>>
>>
>> Hi Chris,
>>
>> I tested your patch on my board sama5d2_xplained, and it works.
>> Thus, you can add my
>> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> However, I disagree with the changes you did in timings. What I found
>> was that timing 'H' could go up to 640 , but timing 'I' to a maximum of
>> 75 or so. [1]
>>
>> I am thinking maybe you could also check your udelays with a scope on
>> the 1wire line ? Because your problem might be in fact in some other
>> part , like udelays not properly aligned/synchronized/accurate at cold
>> boot time, depending on the source of clock you are using.
> 
> I lack a scope, but will extensively test 640 and 75 as the new timings.
> Would that be acceptable?
> 
> Thank you.

Hi Chris,

The timings should be in spec, however, if your particular SoC has a 
problem with delays, this should be investigated.

Does your board with with the maximum timings ? (but still in spec)

Eugen
> 
>>
>> Eugen
>>
>>
>> [1]
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.maximintegrated.com%2Fcontent%2Fdam%2Ffiles%2Fdesign%2Ftools%2Ftech-docs%2F126%2FAN126-timing-calculation.zip&amp;data=04%7C01%7C%7C6787b70951c4494fd76808d9ada98a17%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637731765952771191%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V81HrXrKI8d45WTrvWrc6ydJy2rdCX5%2FL8eYjvxcwCs%3D&amp;reserved=0

Re: [PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Eugen.Hristev]

On 12/9/21 10:27 AM, Eugen Hristev - M18282 wrote:
> On 11/30/21 5:46 PM, Chris Morgan wrote:
>> On Mon, Nov 22, 2021 at 11:16:22AM +0000, Eugen.Hristev@microchip.com wrote:
>>> On 11/8/21 5:07 PM, Chris Morgan wrote:
>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>
>>>> On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
>>>> found. Rebooting on the other hand appears to fix the issue. I found
>>>> that if I modified the timing slightly (but still within spec) the
>>>> w1 identification on cold boot became far more reliable.
>>>>
>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>>> ---
>>>>     drivers/w1/w1-gpio.c | 4 ++--
>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
>>>> index 9346f810ce..5565de2a92 100644
>>>> --- a/drivers/w1/w1-gpio.c
>>>> +++ b/drivers/w1/w1-gpio.c
>>>> @@ -22,8 +22,8 @@
>>>>     #define W1_TIMING_E    9
>>>>     #define W1_TIMING_F    55
>>>>     #define W1_TIMING_G    0
>>>> -#define W1_TIMING_H    480
>>>> -#define W1_TIMING_I    70
>>>> +#define W1_TIMING_H    600
>>>> +#define W1_TIMING_I    100
>>>>     #define W1_TIMING_J    410
>>>>
>>>>     struct w1_gpio_pdata {
>>>> --
>>>> 2.30.2
>>>>
>>>
>>>
>>> Hi Chris,
>>>
>>> I tested your patch on my board sama5d2_xplained, and it works.
>>> Thus, you can add my
>>> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
>>>
>>> However, I disagree with the changes you did in timings. What I found
>>> was that timing 'H' could go up to 640 , but timing 'I' to a maximum of
>>> 75 or so. [1]
>>>
>>> I am thinking maybe you could also check your udelays with a scope on
>>> the 1wire line ? Because your problem might be in fact in some other
>>> part , like udelays not properly aligned/synchronized/accurate at cold
>>> boot time, depending on the source of clock you are using.
>>
>> I lack a scope, but will extensively test 640 and 75 as the new timings.
>> Would that be acceptable?
>>
>> Thank you.
> 
> Hi Chris,
> 
> The timings should be in spec, however, if your particular SoC has a
> problem with delays, this should be investigated.
> 
> Does your board with with the maximum timings ? (but still in spec)


Hi Chris,

I am moving this patch to 'Changes requested' and waiting on your reply 
about 640 / 75 timings which you said you will test.

Eugen

> 
> Eugen
>>
>>>
>>> Eugen
>>>
>>>
>>> [1]
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.maximintegrated.com%2Fcontent%2Fdam%2Ffiles%2Fdesign%2Ftools%2Ftech-docs%2F126%2FAN126-timing-calculation.zip&amp;data=04%7C01%7C%7C6787b70951c4494fd76808d9ada98a17%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637731765952771191%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V81HrXrKI8d45WTrvWrc6ydJy2rdCX5%2FL8eYjvxcwCs%3D&amp;reserved=0
> 

Re: [PATCH] w1: w1-gpio: Loosen timings to improve cold boot reliability

[Chris Morgan]

On Tue, Apr 26, 2022 at 06:49:20AM +0000, Eugen.Hristev@microchip.com wrote:
> On 12/9/21 10:27 AM, Eugen Hristev - M18282 wrote:
> > On 11/30/21 5:46 PM, Chris Morgan wrote:
> >> On Mon, Nov 22, 2021 at 11:16:22AM +0000, Eugen.Hristev@microchip.com wrote:
> >>> On 11/8/21 5:07 PM, Chris Morgan wrote:
> >>>> From: Chris Morgan <macromorgan@hotmail.com>
> >>>>
> >>>> On my NTC CHIP whenever I do a cold boot any attached DIPs cannot be
> >>>> found. Rebooting on the other hand appears to fix the issue. I found
> >>>> that if I modified the timing slightly (but still within spec) the
> >>>> w1 identification on cold boot became far more reliable.
> >>>>
> >>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >>>> ---
> >>>>     drivers/w1/w1-gpio.c | 4 ++--
> >>>>     1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/w1/w1-gpio.c b/drivers/w1/w1-gpio.c
> >>>> index 9346f810ce..5565de2a92 100644
> >>>> --- a/drivers/w1/w1-gpio.c
> >>>> +++ b/drivers/w1/w1-gpio.c
> >>>> @@ -22,8 +22,8 @@
> >>>>     #define W1_TIMING_E    9
> >>>>     #define W1_TIMING_F    55
> >>>>     #define W1_TIMING_G    0
> >>>> -#define W1_TIMING_H    480
> >>>> -#define W1_TIMING_I    70
> >>>> +#define W1_TIMING_H    600
> >>>> +#define W1_TIMING_I    100
> >>>>     #define W1_TIMING_J    410
> >>>>
> >>>>     struct w1_gpio_pdata {
> >>>> --
> >>>> 2.30.2
> >>>>
> >>>
> >>>
> >>> Hi Chris,
> >>>
> >>> I tested your patch on my board sama5d2_xplained, and it works.
> >>> Thus, you can add my
> >>> Tested-by: Eugen Hristev <eugen.hristev@microchip.com>
> >>>
> >>> However, I disagree with the changes you did in timings. What I found
> >>> was that timing 'H' could go up to 640 , but timing 'I' to a maximum of
> >>> 75 or so. [1]
> >>>
> >>> I am thinking maybe you could also check your udelays with a scope on
> >>> the 1wire line ? Because your problem might be in fact in some other
> >>> part , like udelays not properly aligned/synchronized/accurate at cold
> >>> boot time, depending on the source of clock you are using.
> >>
> >> I lack a scope, but will extensively test 640 and 75 as the new timings.
> >> Would that be acceptable?
> >>
> >> Thank you.
> > 
> > Hi Chris,
> > 
> > The timings should be in spec, however, if your particular SoC has a
> > problem with delays, this should be investigated.
> > 
> > Does your board with with the maximum timings ? (but still in spec)
> 
> 
> Hi Chris,
> 
> I am moving this patch to 'Changes requested' and waiting on your reply 
> about 640 / 75 timings which you said you will test.

I tested. It works better, but still fails some of the time on cold boot
(warm boots always seem to succeed though). For some reason so far I am
only able to get the original timings I submitted to work consistently.

I have tested this on both an NTC CHIP and a prototype Source Parts
Popcorn, both with similar effect (warm boot fine, cold boot
intermittant). Both boards use the same SOC and are tested with the
same DIP (a PocketCHIP).

If this needs to be made board specific to overcome an issue with the
Allwinner R8 let me know and maybe we can figure out a board specific
override.

Thank you.

> 
> Eugen
> 
> > 
> > Eugen
> >>
> >>>
> >>> Eugen
> >>>
> >>>
> >>> [1]
> >>> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.maximintegrated.com%2Fcontent%2Fdam%2Ffiles%2Fdesign%2Ftools%2Ftech-docs%2F126%2FAN126-timing-calculation.zip&amp;data=05%7C01%7C%7Cb5e9eb5aec71479346f408da2750e636%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637865525660163935%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=T6wdIVwoDUy5MDuTIe19giBACfUD5Xfwx5E7JjA0P6A%3D&amp;reserved=0
> > 
>