[v4,0/3] Add support for WDIOF_CARDRESET on TI AM65x

Message

Li, Hua Qian July 17, 2023, 4:07 a.m. UTC

From: Li Hua Qian <[e-mail]>

The watchdog hardware of TI AM65X platform does not support
WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
reset cause, to know if the board reboot is due to a watchdog reset.

Signed-off-by: Li Hua Qian <[e-mail]>
---
Changes in v4:
- Fix the coding style.
- Add usage note for the reserved memory.
- Link to v3:
  https://lore.kernel.org/linux-watchdog/[e-mail]

Changes in v3:
- Add memory-region back for the reserved memory, and remove reserved
  memory from the watchdog IO address space.
- Add changelog.
- Link to v2:
  https://lore.kernel.org/linux-watchdog/[e-mail]

Changes in v2:
- Remove memory-region and memory-size properties, and bind the reserved
  memory to watchdog IO address space.
- Remove the unnecessary rti_wdt_ioctl.
- Fix the mail list
- Link to v1:
  https://lore.kernel.org/all/[e-mail]
  v1 had a wrong mail list at the beginning, and the mail thread was
  messed up.

Li Hua Qian (3):
  dt-bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET
  arm64: dts: ti: Add reserved memory for watchdog
  watchdog:rit_wdt: Add support for WDIOF_CARDRESET

 .../bindings/watchdog/ti,rti-wdt.yaml         | 41 ++++++++++++++++
 .../boot/dts/ti/k3-am65-iot2050-common.dtsi   | 10 ++++
 drivers/watchdog/rti_wdt.c                    | 48 +++++++++++++++++++
 3 files changed, 99 insertions(+)

Comments

Krzysztof Kozlowski July 17, 2023, 6:16 a.m. UTC | #1

On 17/07/2023 06:07, [e-mail] wrote:
> From: Li Hua Qian <[e-mail]>
> 
> The watchdog hardware of TI AM65X platform does not support
> WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
> reset cause, to know if the board reboot is due to a watchdog reset.
> 
> Signed-off-by: Li Hua Qian <[e-mail]>
> ---
> Changes in v4:
> - Fix the coding style.
> - Add usage note for the reserved memory.
> - Link to v3:
>   https://lore.kernel.org/linux-watchdog/[e-mail]

Much more changed. You added example in the bindings which no one asked
for. Then you added multiple fake review tags to all the patches.

Best regards,
Krzysztof

Li, Hua Qian July 17, 2023, 6:24 a.m. UTC | #2

On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> On 17/07/2023 06:07, [e-mail] wrote:
> > From: Li Hua Qian <[e-mail]>
> > 
> > The watchdog hardware of TI AM65X platform does not support
> > WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
> > reset cause, to know if the board reboot is due to a watchdog
> > reset.
> > 
> > Signed-off-by: Li Hua Qian <[e-mail]>
> > ---
> > Changes in v4:
> > - Fix the coding style.
> > - Add usage note for the reserved memory.
> > - Link to v3:
> >  
> > https://lore.kernel.org/linux-watchdog/[e-mail]
> 
> Much more changed. You added example in the bindings which no one
> asked
> for. Then you added multiple fake review tags to all the patches.
> 
> Best regards,
> Krzysztof
> 
Hi,

Sorry for the wrong statement. I missed some key information and
missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
has reviewed`.

Best regards,
Li Hua Qian

Krzysztof Kozlowski July 17, 2023, 6:27 a.m. UTC | #3

On 17/07/2023 08:24, Li, Hua Qian wrote:
> On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
>> On 17/07/2023 06:07, [e-mail] wrote:
>>> From: Li Hua Qian <[e-mail]>
>>>
>>> The watchdog hardware of TI AM65X platform does not support
>>> WDIOF_CARDRESET feature, add a reserved memory to save the watchdog
>>> reset cause, to know if the board reboot is due to a watchdog
>>> reset.
>>>
>>> Signed-off-by: Li Hua Qian <[e-mail]>
>>> ---
>>> Changes in v4:
>>> - Fix the coding style.
>>> - Add usage note for the reserved memory.
>>> - Link to v3:
>>>  
>>> https://lore.kernel.org/linux-watchdog/[e-mail]
>>
>> Much more changed. You added example in the bindings which no one
>> asked
>> for. Then you added multiple fake review tags to all the patches.
>>
>> Best regards,
>> Krzysztof
>>
> Hi,
> 
> Sorry for the wrong statement. I missed some key information and
> missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> has reviewed`.

But you don't have even that information who has reviewed! Where do you
see any reviews coming from me for patch #2? Where do you see reviews
from Rob for patch #3?

Best regards,
Krzysztof

Li, Hua Qian July 17, 2023, 6:28 a.m. UTC | #4

On Mon, 2023-07-17 at 14:23 +0800, Hua Qian Li wrote:
> On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> > On 17/07/2023 06:07, [e-mail] wrote:
> > > From: Li Hua Qian <[e-mail]>
> > > 
> > > The watchdog hardware of TI AM65X platform does not support
> > > WDIOF_CARDRESET feature, add a reserved memory to save the
> > > watchdog
> > > reset cause, to know if the board reboot is due to a watchdog
> > > reset.
> > > 
> > > Signed-off-by: Li Hua Qian <[e-mail]>
> > > ---
> > > Changes in v4:
> > > - Fix the coding style.
> > > - Add usage note for the reserved memory.
> > > - Link to v3:
> > >  
> > > https://lore.kernel.org/linux-watchdog/[e-mail]
> > 
> > Much more changed. You added example in the bindings which no one
> > asked
> > for. Then you added multiple fake review tags to all the patches.
> > 
> > Best regards,
> > Krzysztof
> > 
> Hi,
> 
> Sorry for the wrong statement. I missed some key information and
> missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> has reviewed`.
> 
> Best regards,
> Li Hua Qian
By the way, `Much more changes` came from the following message which
was commented on [PATCH v3 0/3] Add support for WDIOF_CARDRESET on TI
AM65x:


One thing I keep wondering about: What prevents the Linux kernel from
treating the special memory area like normal memory ? I would have
expected
some usage note, such as that the memory area must be reported as
reserved
to the kernel, but I don't see anything like that.

Guenter

Li, Hua Qian July 17, 2023, 6:34 a.m. UTC | #5

On Mon, 2023-07-17 at 08:27 +0200, Krzysztof Kozlowski wrote:
> On 17/07/2023 08:24, Li, Hua Qian wrote:
> > On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> > > On 17/07/2023 06:07, [e-mail] wrote:
> > > > From: Li Hua Qian <[e-mail]>
> > > > 
> > > > The watchdog hardware of TI AM65X platform does not support
> > > > WDIOF_CARDRESET feature, add a reserved memory to save the
> > > > watchdog
> > > > reset cause, to know if the board reboot is due to a watchdog
> > > > reset.
> > > > 
> > > > Signed-off-by: Li Hua Qian <[e-mail]>
> > > > ---
> > > > Changes in v4:
> > > > - Fix the coding style.
> > > > - Add usage note for the reserved memory.
> > > > - Link to v3:
> > > >  
> > > > https://lore.kernel.org/linux-watchdog/[e-mail]
> > > 
> > > Much more changed. You added example in the bindings which no one
> > > asked
> > > for. Then you added multiple fake review tags to all the patches.
> > > 
> > > Best regards,
> > > Krzysztof
> > > 
> > Hi,
> > 
> > Sorry for the wrong statement. I missed some key information and
> > missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> > has reviewed`.
> 
> But you don't have even that information who has reviewed! Where do
> you
> see any reviews coming from me for patch #2? Where do you see reviews
> from Rob for patch #3?
> 
> Best regards,
> Krzysztof
> 
I got these information from my email thread. Anyway I made a stupid
mistake, sorry for wasting your time.

By the way, when you wrote the following in '[PATCH v3 1/3] dt-
bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET', you
were kind of saying that it looks good to you if I remove the extra
empty line, right?

In any case:
Reviewed-by: Krzysztof Kozlowski <[e-mail]>

Best regards,
Li Hua Qian

Krzysztof Kozlowski July 17, 2023, 6:43 a.m. UTC | #6

On 17/07/2023 08:34, Li, Hua Qian wrote:
> On Mon, 2023-07-17 at 08:27 +0200, Krzysztof Kozlowski wrote:
>> On 17/07/2023 08:24, Li, Hua Qian wrote:
>>> On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
>>>> On 17/07/2023 06:07, [e-mail] wrote:
>>>>> From: Li Hua Qian <[e-mail]>
>>>>>
>>>>> The watchdog hardware of TI AM65X platform does not support
>>>>> WDIOF_CARDRESET feature, add a reserved memory to save the
>>>>> watchdog
>>>>> reset cause, to know if the board reboot is due to a watchdog
>>>>> reset.
>>>>>
>>>>> Signed-off-by: Li Hua Qian <[e-mail]>
>>>>> ---
>>>>> Changes in v4:
>>>>> - Fix the coding style.
>>>>> - Add usage note for the reserved memory.
>>>>> - Link to v3:
>>>>>  
>>>>> https://lore.kernel.org/linux-watchdog/[e-mail]
>>>>
>>>> Much more changed. You added example in the bindings which no one
>>>> asked
>>>> for. Then you added multiple fake review tags to all the patches.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> Hi,
>>>
>>> Sorry for the wrong statement. I missed some key information and
>>> missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
>>> has reviewed`.
>>
>> But you don't have even that information who has reviewed! Where do
>> you
>> see any reviews coming from me for patch #2? Where do you see reviews
>> from Rob for patch #3?
>>
>> Best regards,
>> Krzysztof
>>
> I got these information from my email thread. Anyway I made a stupid
> mistake, sorry for wasting your time.
> 
> By the way, when you wrote the following in '[PATCH v3 1/3] dt-
> bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET', you
> were kind of saying that it looks good to you if I remove the extra
> empty line, right?
> 
> In any case:
> Reviewed-by: Krzysztof Kozlowski <[e-mail]>

This was patch 1. But you added my review to patch 2 also. Why then not
adding to patch 3? What logic is driving this?

Best regards,
Krzysztof

Li, Hua Qian July 17, 2023, 6:49 a.m. UTC | #7

On Mon, 2023-07-17 at 08:43 +0200, Krzysztof Kozlowski wrote:
> On 17/07/2023 08:34, Li, Hua Qian wrote:
> > On Mon, 2023-07-17 at 08:27 +0200, Krzysztof Kozlowski wrote:
> > > On 17/07/2023 08:24, Li, Hua Qian wrote:
> > > > On Mon, 2023-07-17 at 08:16 +0200, Krzysztof Kozlowski wrote:
> > > > > On 17/07/2023 06:07, [e-mail] wrote:
> > > > > > From: Li Hua Qian <[e-mail]>
> > > > > > 
> > > > > > The watchdog hardware of TI AM65X platform does not support
> > > > > > WDIOF_CARDRESET feature, add a reserved memory to save the
> > > > > > watchdog
> > > > > > reset cause, to know if the board reboot is due to a
> > > > > > watchdog
> > > > > > reset.
> > > > > > 
> > > > > > Signed-off-by: Li Hua Qian <[e-mail]>
> > > > > > ---
> > > > > > Changes in v4:
> > > > > > - Fix the coding style.
> > > > > > - Add usage note for the reserved memory.
> > > > > > - Link to v3:
> > > > > >  
> > > > > > https://lore.kernel.org/linux-watchdog/[e-mail]
> > > > > 
> > > > > Much more changed. You added example in the bindings which no
> > > > > one
> > > > > asked
> > > > > for. Then you added multiple fake review tags to all the
> > > > > patches.
> > > > > 
> > > > > Best regards,
> > > > > Krzysztof
> > > > > 
> > > > Hi,
> > > > 
> > > > Sorry for the wrong statement. I missed some key information
> > > > and
> > > > missunderstood `Reviewed-by`, I treated `Reviewed-by` as `Who
> > > > has reviewed`.
> > > 
> > > But you don't have even that information who has reviewed! Where
> > > do
> > > you
> > > see any reviews coming from me for patch #2? Where do you see
> > > reviews
> > > from Rob for patch #3?
> > > 
> > > Best regards,
> > > Krzysztof
> > > 
> > I got these information from my email thread. Anyway I made a
> > stupid
> > mistake, sorry for wasting your time.
> > 
> > By the way, when you wrote the following in '[PATCH v3 1/3] dt-
> > bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET',
> > you
> > were kind of saying that it looks good to you if I remove the extra
> > empty line, right?
> > 
> > In any case:
> > Reviewed-by: Krzysztof Kozlowski <[e-mail]>
> 
> This was patch 1. But you added my review to patch 2 also. Why then
> not
> adding to patch 3? What logic is driving this?
> 
> Best regards,
> Krzysztof
> 
Much sorry again, please forget the wrong info in v4. I wrongly added
you to patch 2 only because I found you commented on patch 2 once.

Best regards,
Li Hua Qian