Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

[Guenter Roeck]

On 5/17/24 11:00, Guenter Roeck wrote:

On 5/17/24 10:48, Steven Rostedt wrote:

On Fri, 17 May 2024 10:36:37 -0700
Guenter Roeck  wrote:


Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
  from include/trace/define_trace.h:102,
  from drivers/cxl/core/trace.h:737,
  from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2 
arguments, but takes just 1

This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.


Looks like I made this patch just before this commit was pulled into
Linus's tree.

Which is why I'll apply and rerun the above again probably on Tuesday of
next week against Linus's latest.

This patch made it through both an allyesconfig and an allmodconfig, but on
the commit I had applied it to, which was:

   1b294a1f3561 ("Merge tag 'net-next-6.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")

I'll be compiling those two builds after I update it then.



I am currently repeating my test builds with the above errors fixed.
That should take a couple of hours. I'll let you know how it goes.



There are no more build failures caused by this patch after fixing the above
errors.

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

[Guenter Roeck]

On 5/17/24 10:48, Steven Rostedt wrote:

On Fri, 17 May 2024 10:36:37 -0700
Guenter Roeck  wrote:


Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
  from include/trace/define_trace.h:102,
  from drivers/cxl/core/trace.h:737,
  from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2 
arguments, but takes just 1

This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.


Looks like I made this patch just before this commit was pulled into
Linus's tree.

Which is why I'll apply and rerun the above again probably on Tuesday of
next week against Linus's latest.

This patch made it through both an allyesconfig and an allmodconfig, but on
the commit I had applied it to, which was:

   1b294a1f3561 ("Merge tag 'net-next-6.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")

I'll be compiling those two builds after I update it then.



I am currently repeating my test builds with the above errors fixed.
That should take a couple of hours. I'll let you know how it goes.

Guenter

Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

[Guenter Roeck]

On Thu, May 16, 2024 at 01:34:54PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" 
> 
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
> 
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
> 
> This means that with:
> 
>   __string(field, mystring)
> 
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
> 
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
> 
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
> 
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
> 

Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
 from include/trace/define_trace.h:102,
 from drivers/cxl/core/trace.h:737,
 from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2 
arguments, but takes just 1

This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.

Guenter

Re: [PATCH 64/64] i2c: reword i2c_algorithm in drivers according to newest specification

[Guenter Roeck]

On Fri, Mar 22, 2024 at 9:47 AM Wolfram Sang
 wrote:
>
>
> > Acked-by: Nicolas Ferre  # for at91
> > Probably file names themselves will need some care, in a second time.
>
> Totally true. I am aware of that. But one step after the other...
>

Kind of odd though to change function names but not parameter names of
those very same functions.

Guenter

Re: [PATCH v1] lib,kprobes: using try_cmpxchg_local in objpool_push

[Guenter Roeck]

On Mon, Oct 23, 2023 at 07:24:52PM +0800, wuqiang.matt wrote:
> The objpool_push can only happen on local cpu node, so only the local
> cpu can touch slot->tail and slot->last, which ensures the correctness
> of using cmpxchg without lock prefix (using try_cmpxchg_local instead
> of try_cmpxchg_acquire).
> 
> Testing with IACA found the lock version of pop/push pair costs 16.46
> cycles and local-push version costs 15.63 cycles. Kretprobe throughput
> is improved to 1.019 times of the lock version for x86_64 systems.
> 
> OS: Debian 10 X86_64, Linux 6.6rc6 with freelist
> HW: XEON 8336C x 2, 64 cores/128 threads, DDR4 3200MT/s
> 
>  1T 2T 4T 8T16T
>   lock:29909085   59865637  119692073  239750369  478005250
>   local:   30297523   60532376  121147338  242598499  484620355
> 32T48T64T96T   128T
>   lock:   957553042 1435814086 1680872925 2043126796 2165424198
>   local:  968526317 1454991286 1861053557 2059530343 2171732306
> 
> Signed-off-by: wuqiang.matt 

This patch results in

lib/objpool.c:169:12: error: implicit declaration of function 
'arch_cmpxchg_local' is invalid in C99

or

lib/objpool.c: In function 'objpool_try_add_slot':
include/linux/atomic/atomic-arch-fallback.h:384:27: error: implicit declaration 
of function 'arch_cmpxchg_local'

for various architectures (I have seen it with arc, hexagon, and openrisc
so far).

As usual, my apologies for the noise if this has already been reported
and/or fixed.

Guenter

Re: [PATCH 1/3] dt-bindings: watchdog: qcom-wdt: Add MSM8226 and MSM8974 compatibles

[Guenter Roeck]

On Wed, Oct 11, 2023 at 06:33:13PM +0200, Luca Weiss wrote:
> From: Matti Lehtimäki 
> 
> Add compatibles for the MSM8226 and MSM8974 platforms to the Qualcomm
> watchdog binding.
> 
> Signed-off-by: Matti Lehtimäki 
> Signed-off-by: Luca Weiss 

Reviewed-by: Guenter Roeck 

> ---
>  Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml 
> b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> index 5046dfa55f13..c12bc852aedc 100644
> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml
> @@ -21,6 +21,8 @@ properties:
>- qcom,apss-wdt-ipq5018
>- qcom,apss-wdt-ipq5332
>- qcom,apss-wdt-ipq9574
> +  - qcom,apss-wdt-msm8226
> +  - qcom,apss-wdt-msm8974
>- qcom,apss-wdt-msm8994
>- qcom,apss-wdt-qcm2290
>- qcom,apss-wdt-qcs404
> 
> -- 
> 2.42.0
> 

Re: [PATCH] MIPS: pci-legacy: revert "use generic pci_enable_resources"

[Guenter Roeck]

On Mon, Apr 19, 2021 at 11:39:43PM -0700, Ilya Lipnitskiy wrote:
> This mostly reverts commit 99bca615d895 ("MIPS: pci-legacy: use generic
> pci_enable_resources"). Fixes regressions such as:
>   ata_piix :00:0a.1: can't enable device: BAR 0 [io  0x01f0-0x01f7] not
>   claimed
>   ata_piix: probe of :00:0a.1 failed with error -22
> 
> The only changes from the strict revert are to fix checkpatch errors:
>   ERROR: spaces required around that '=' (ctx:VxV)
>   #33: FILE: arch/mips/pci/pci-legacy.c:252:
>   +   for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
>   ^
> 
>   ERROR: do not use assignment in if condition
>   #67: FILE: arch/mips/pci/pci-legacy.c:284:
>   +   if ((err = pcibios_enable_resources(dev, mask)) < 0)
> 
> Reported-by: Guenter Roeck 
> Signed-off-by: Ilya Lipnitskiy 

Tested-by: Guenter Roeck 

Guenter

Re: Enabling pmbus power control

[Guenter Roeck]

On 4/20/21 12:06 AM, Zev Weiss wrote:
> On Tue, Apr 20, 2021 at 01:00:25AM CDT, Guenter Roeck wrote:
>> On 4/19/21 10:50 PM, Zev Weiss wrote:
>> [ ... ]
>>
>>> I had a glance at the enclosure driver; it looks pretty geared toward 
>>> SES-like things (drivers/scsi/ses.c being its only usage I can see in the 
>>> kernel at the moment) and while it could perhaps be pressed into working 
>>> for this it seems like it would probably drag in a fair amount of 
>>> boilerplate and result in a somewhat gratuitously confusing driver 
>>> arrangement (calling the things involved in the cases we're looking at 
>>> "enclosures" seems like a bit of a stretch).
>>>
>>> As an alternative, would something like the patch below be more along the 
>>> lines of what you're suggesting?  And if so, would it make sense to 
>>> generalize it into something like 'pmbus-switch.c' and add a 
>>> PMBUS_HAVE_POWERSWITCH functionality bit or similar in the pmbus code 
>>> instead of hardcoding it for only LM25066 support?
>>>
>>>
>>
>> No. Don't access pmbus functions from outside drivers/hwmon/pmbus.
>>
>> I used to be opposed to function export restrictions (aka export namespaces),
>> but you are making a good case that we need to introduce them for pmbus
>> functions.
>>
>> Guenter
> 
> Okay -- I figured that was likely to be frowned upon, but the alternative 
> seemed to be effectively duplicating non-trivial chunks of the pmbus code.  
> However, upon realizing that the LM25066 doesn't actually require any of the 
> paging work the generic pmbus code provides, I suppose it can actually be 
> done with a simple smbus read & write.  Does this version look better?
> 

It is just getting worse and worse. You are re-implementing regulator
support for the lm25066. The correct solution would be to implement
regulator support in the lm25066 and use it from the secondary driver
(which should be chip independent).

Guenter

> 
> Zev
> 
> 
> From 1662e1c59c498ad6b208e6ab450bd467d71def34 Mon Sep 17 00:00:00 2001
> From: Zev Weiss 
> Date: Wed, 31 Mar 2021 01:58:35 -0500
> Subject: [PATCH] misc: add lm25066-switch driver
> 
> This driver allows an lm25066 to be switched on and off from userspace
> via sysfs.
> 
> Signed-off-by: Zev Weiss 
> ---
>  drivers/misc/Kconfig  |   7 ++
>  drivers/misc/Makefile |   1 +
>  drivers/misc/lm25066-switch.c | 126 ++
>  3 files changed, 134 insertions(+)
>  create mode 100644 drivers/misc/lm25066-switch.c
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f532c59bb59b..384b6022ec15 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -445,6 +445,13 @@ config HISI_HIKEY_USB
>    switching between the dual-role USB-C port and the USB-A host ports
>    using only one USB controller.
>  
> +config LM25066_SWITCH
> +    tristate "LM25066 power switch support"
> +    depends on OF && SENSORS_LM25066
> +    help
> +  This driver augments LM25066 hwmon support with power switch
> +  functionality controllable from userspace via sysfs.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 99b6f15a3c70..c948510d0cc9 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI)    += habanalabs/
>  obj-$(CONFIG_UACCE)    += uacce/
>  obj-$(CONFIG_XILINX_SDFEC)    += xilinx_sdfec.o
>  obj-$(CONFIG_HISI_HIKEY_USB)    += hisi_hikey_usb.o
> +obj-$(CONFIG_LM25066_SWITCH)    += lm25066-switch.o
> diff --git a/drivers/misc/lm25066-switch.c b/drivers/misc/lm25066-switch.c
> new file mode 100644
> index ..9adc67c320f9
> --- /dev/null
> +++ b/drivers/misc/lm25066-switch.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This module provides a thin wrapper around the lm25066 hwmon driver that
> + * additionally exposes a userspace-controllable on/off power switch via
> + * sysfs.
> + *
> + * Author: Zev Weiss 
> + *
> + * Copyright (C) 2021 Equinix Services, Inc.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * The relevant PMBus command and data values for controlling the LM25066
> + * power state.  Because it's not a paged device we skip the usual paging
> + * business other PMBus devices might require.
> + */
> +#define CMD_OPERATION 0x01
> +#de

Re: Enabling pmbus power control

[Guenter Roeck]

On 4/19/21 10:50 PM, Zev Weiss wrote:
[ ... ]

> I had a glance at the enclosure driver; it looks pretty geared toward 
> SES-like things (drivers/scsi/ses.c being its only usage I can see in the 
> kernel at the moment) and while it could perhaps be pressed into working for 
> this it seems like it would probably drag in a fair amount of boilerplate and 
> result in a somewhat gratuitously confusing driver arrangement (calling the 
> things involved in the cases we're looking at "enclosures" seems like a bit 
> of a stretch).
> 
> As an alternative, would something like the patch below be more along the 
> lines of what you're suggesting?  And if so, would it make sense to 
> generalize it into something like 'pmbus-switch.c' and add a 
> PMBUS_HAVE_POWERSWITCH functionality bit or similar in the pmbus code instead 
> of hardcoding it for only LM25066 support?
> 
> 

No. Don't access pmbus functions from outside drivers/hwmon/pmbus.

I used to be opposed to function export restrictions (aka export namespaces),
but you are making a good case that we need to introduce them for pmbus
functions.

Guenter

Re: [PATCH v2 8/8] MIPS: pci-legacy: use generic pci_enable_resources

[Guenter Roeck]

On Tue, Apr 13, 2021 at 08:12:40PM -0700, Ilya Lipnitskiy wrote:
> Follow the reasoning from commit 842de40d93e0 ("PCI: add generic
> pci_enable_resources()"):
> 
>   The only functional difference from the MIPS version is that the
>   generic one uses "!r->parent" to check for resource collisions
>   instead of "!r->start && r->end".
> 
> That should have no effect on any pci-legacy driver.
> 

Unfortunately it does. With this patch in place, all my mips qemu emulations
fail to boot from ide/ata drive; the driver is not instantiated. The error
message is:

ata_piix :00:0a.1: can't enable device: BAR 0 [io  0x01f0-0x01f7] not 
claimed
ata_piix: probe of :00:0a.1 failed with error -22

Reverting this patch fixes the problem, and the message displayed by the driver
is:

ata_piix :00:0a.1: enabling device ( -> 0001)

Bisect log is attached.

Guenter

---
# bad: [50b8b1d699ac313c0a07a3c185ffb23aecab8abb] Add linux-next specific files 
for 20210419
# good: [bf05bf16c76bb44ab5156223e1e58e26dfe30a88] Linux 5.12-rc8
git bisect start 'HEAD' 'v5.12-rc8'
# bad: [c4bb91fc07e59241cde97f913d7a2fbedc248f0d] Merge remote-tracking branch 
'crypto/master'
git bisect bad c4bb91fc07e59241cde97f913d7a2fbedc248f0d
# bad: [499f739ad70f2a58aac985dceb25ca7666da88be] Merge remote-tracking branch 
'jc_docs/docs-next'
git bisect bad 499f739ad70f2a58aac985dceb25ca7666da88be
# bad: [11b56408a328d1c5c4dfa7667c5dc46956b64aec] Merge remote-tracking branch 
'parisc-hd/for-next'
git bisect bad 11b56408a328d1c5c4dfa7667c5dc46956b64aec
# good: [09ccc0ee1227f2cfe50d8dbbe241d115d9b3885f] Merge branch 'arm/defconfig' 
into for-next
git bisect good 09ccc0ee1227f2cfe50d8dbbe241d115d9b3885f
# good: [a5b76c2f17330e266a5c56dde21430e27b0d0dbb] Merge remote-tracking branch 
'arm-soc/for-next'
git bisect good a5b76c2f17330e266a5c56dde21430e27b0d0dbb
# good: [1e4241f6813f1c1a0027d96df075ffd01808b3cf] Merge remote-tracking branch 
'ti-k3/ti-k3-next'
git bisect good 1e4241f6813f1c1a0027d96df075ffd01808b3cf
# good: [7496a43be7a362391607d78e49a3f28de80029ce] Merge remote-tracking branch 
'h8300/h8300-next'
git bisect good 7496a43be7a362391607d78e49a3f28de80029ce
# good: [66633abd0642f1e89d26e15f36fb13d3a1c535ff] MIPS/bpf: Enable 
bpf_probe_read{, str}() on MIPS again
git bisect good 66633abd0642f1e89d26e15f36fb13d3a1c535ff
# good: [2c92ef8ff8d327797c1920ae7f938bcc6f3f7421] MIPS: Fix strnlen_user 
access check
git bisect good 2c92ef8ff8d327797c1920ae7f938bcc6f3f7421
# good: [3a070801c61f4e3987d59b1068368ba71d727208] Merge remote-tracking branch 
'microblaze/next'
git bisect good 3a070801c61f4e3987d59b1068368ba71d727208
# good: [317f553bb677e324c9c865ff7f14597bc5ceeb9c] MIPS: pci-legacy: remove 
redundant info messages
git bisect good 317f553bb677e324c9c865ff7f14597bc5ceeb9c
# bad: [6ce48897ce476bed86fde28752c27596e8753277] MIPS: Loongson64: Add 
kexec/kdump support
git bisect bad 6ce48897ce476bed86fde28752c27596e8753277
# bad: [99bca615d89510917864fac6b26fd343eff2aba2] MIPS: pci-legacy: use generic 
pci_enable_resources
git bisect bad 99bca615d89510917864fac6b26fd343eff2aba2
# good: [0af83d2e447af3e5098583cb6320bb1b1fb0976b] MIPS: pci-legacy: remove 
busn_resource field
git bisect good 0af83d2e447af3e5098583cb6320bb1b1fb0976b
# first bad commit: [99bca615d89510917864fac6b26fd343eff2aba2] MIPS: 
pci-legacy: use generic pci_enable_resources

Re: Enabling pmbus power control

[Guenter Roeck]

On Mon, Apr 19, 2021 at 08:29:53PM -0500, Zev Weiss wrote:
> On Tue, Mar 30, 2021 at 02:38:10PM CDT, Guenter Roeck wrote:
> > On Tue, Mar 30, 2021 at 07:02:00PM +0100, Mark Brown wrote:
> > > On Tue, Mar 30, 2021 at 12:56:56PM -0500, Zev Weiss wrote:
> > > 
> > > > Okay, to expand a bit on the description in my initial message -- we've
> > > > got a single chassis with multiple server boards and a single manager 
> > > > board
> > > > that handles, among other things, power control for the servers.
> > > > The manager board has one LM25066 for each attached server, which acts 
> > > > as
> > > > the "power switch" for that server.  There thus really isn't any driver 
> > > > to
> > > > speak of for the downstream device.
> > > 
> > > This sounds like you need a driver representing those server boards (or
> > > the slots they plug into perhaps) that represents everything about those
> > > boards to userspace, including power switching.  I don't see why you
> > > wouldn't have a driver for that - it's a thing that physically exists
> > > and clearly has some software control, and you'd presumably also expect
> > > to represent some labelling about the slot as well.
> > 
> > Absolutely agree.
> > 
> > Thanks,
> > Guenter
> 
> Hi Guenter, Mark,
> 
> I wanted to return to this to try to explain why structuring the kernel
> support for this in a way that's specific to the device behind the PMIC
> seems like an awkward fit to me, and ultimately kind of artificial.
> 
> In the system I described, the manager board with the LM25066 devices is its
> own little self-contained BMC system running its own Linux kernel (though
> "BMC" is perhaps a slightly misleading term because there's no host system
> that it manages).  The PMICs are really the only relevant connection it has
> to the servers it controls power for -- they have their own dedicated local
> BMCs on board as well doing all the usual BMC tasks.  A hypothetical
> dedicated driver for this on the manager board wouldn't have any other
> hardware to touch aside from the pmbus interface of the LM25066 itself, so
> calling it a server board driver seems pretty arbitrary -- and in fact the
> same system also has another LM25066 that controls the power supply to the
> chassis cooling fans (a totally different downstream device, but one that
> would be equally well-served by the same software).
> 
> More recently, another system has entered the picture for us that might
> illustrate it more starkly -- the Delta Open19 power shelf [0] supported by
> a recent code release from LinkedIn [1].  This is a rackmount power supply

All I can see is that this code is a mess.

> with fifty outputs, each independently switchable via an LM25066 attached to
> an on-board BMC-style management controller (though again, no host system
> involved).  We (Equinix Metal) are interested in porting a modern OpenBMC to
> it to replace the dated, crufty, pre-Linux-Foundation version of OpenBMC it
> currently runs (as found in the linked repo).  The exact nature of the
> devices powered by its outputs is well outside the scope of the firmware
> running on that controller (it could be any arbitrary thing that runs on
> 12VDC), but we still want to be able to both (a) retrieve per-output
> voltage/current/power readings as provided by the existing LM25066 hwmon
> support, and (b) control the on/off state of those outputs from userspace.
> 
> Given the array of possible use-cases, an approach of adding power-switch
> functionality to the existing LM25066 support seems like the most obvious
> way to support this, so I'm hoping to see if I can get some idea of what an
> acceptable implementation of that might look like.
> 

Sorry, that is simply a no-go for the LM25066 driver. I mentioned it before,
and it is still true: The hwmon subsystem is not in the business of power
control.

Have you looked into enclosures (drivers/misc/enclosure.c) ? Maybe that
or something similar could be used for your purposes.

Thanks,
Guenter

Re: [PATCH 5.11 000/122] 5.11.16-rc1 review

[Guenter Roeck]

On Mon, Apr 19, 2021 at 03:04:40PM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.11.16 release.
> There are 122 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 21 Apr 2021 13:05:09 +.
> Anything received after that time might be too late.
> 

Build results:
total: 155 pass: 155 fail: 0
Qemu test results:
total: 461 pass: 461 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 5.4 00/73] 5.4.114-rc1 review

[Guenter Roeck]

On Mon, Apr 19, 2021 at 03:05:51PM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.4.114 release.
> There are 73 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 21 Apr 2021 13:05:09 +.
> Anything received after that time might be too late.
> 

Build results:
total: 157 pass: 157 fail: 0
Qemu test results:
total: 433 pass: 433 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH] hwmon: (pmbus/max15301) Add pmbus driver for MAX15301

[Guenter Roeck]

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aa84121c5611..de2ad7223055 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10790,6 +10790,13 @@ S:   Orphan
>  F:   drivers/video/fbdev/matrox/matroxfb_*
>  F:   include/uapi/linux/matroxfb.h
>  
> +MAX15301 DRIVER
> +M:   Daniel Nilsson 
> +L:   linux-hw...@vger.kernel.org
> +S:   Maintained
> +F:   Documentation/hwmon/max15301.rst
> +F:   drivers/hwmon/pmbus/max15301.c
> +
>  MAX16065 HARDWARE MONITOR DRIVER
>  M:   Guenter Roeck 
>  L:   linux-hw...@vger.kernel.org
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 32d2fc850621..5c9fb1a88cec 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -148,6 +148,15 @@ config SENSORS_LTC3815
> This driver can also be built as a module. If so, the module will
> be called ltc3815.
>  
> +config SENSORS_MAX15301
> + tristate "Maxim MAX15301"
> + help
> +   If you say yes here you get hardware monitoring support for Maxim
> +   MAX15301, as well as for Flex BMR461.
> +
> +   This driver can also be built as a module. If so, the module will
> +   be called max15301.
> +
>  config SENSORS_MAX16064
>   tristate "Maxim MAX16064"
>   help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 6a4ba0fdc1db..6040bc8718e9 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_SENSORS_ISL68137)  += isl68137.o
>  obj-$(CONFIG_SENSORS_LM25066)+= lm25066.o
>  obj-$(CONFIG_SENSORS_LTC2978)+= ltc2978.o
>  obj-$(CONFIG_SENSORS_LTC3815)+= ltc3815.o
> +obj-$(CONFIG_SENSORS_MAX15301)   += max15301.o
>  obj-$(CONFIG_SENSORS_MAX16064)   += max16064.o
>  obj-$(CONFIG_SENSORS_MAX16601)   += max16601.o
>  obj-$(CONFIG_SENSORS_MAX20730)   += max20730.o
> diff --git a/drivers/hwmon/pmbus/max15301.c b/drivers/hwmon/pmbus/max15301.c
> new file mode 100644
> index ..eb9b7a5ef052
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/max15301.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for Maxim MAX15301
> + *
> + * Copyright (c) 2021 Flextronics International Sweden AB
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "pmbus.h"
> +
> +static const struct i2c_device_id max15301_id[] = {
> + {"bmr461", 0},
> + {"max15301", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, max15301_id);
> +
> +struct max15301_data {
> + int id;
> + ktime_t access; /* Chip access time */
> + int delay;  /* Delay between chip accesses in us */
> + struct pmbus_driver_info info;
> +};
> +
> +#define to_max15301_data(x)  container_of(x, struct max15301_data, info)
> +
> +#define MAX15301_WAIT_TIME   100 /* us   */
> +
> +static ushort delay = MAX15301_WAIT_TIME;
> +module_param(delay, ushort, 0644);
> +MODULE_PARM_DESC(delay, "Delay between chip accesses in us");
> +
> +static struct max15301_data max15301_data = {
> + .info = {
> + .pages = 1,
> + .func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> + | PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
> + | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
> + | PMBUS_HAVE_STATUS_TEMP
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT,
> + }
> +};
> +
> +/* This chip needs a delay between accesses */
> +static inline void max15301_wait(const struct max15301_data *data)
> +{
> + if (data->delay) {
> + s64 delta = ktime_us_delta(ktime_get(), data->access);
> +
> + if (delta < data->delay)
> + udelay(data->delay - delta);
> + }
> +}
> +
> +static int max15301_read_word_data(struct i2c_client *client, int page,
> +int phase, int reg)
> +{
> + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> + struct max15301_data *data = to_max15301_data(info);
> + int ret;
> +
> + if (page > 0)
> + return -ENXIO;
> +
> + if (reg >= PMBUS_VIRT_BASE)
> + return -ENXIO;
> +
> + max15301_wait(data);
> + ret = pmbus_read_word_data(client, page, phase, reg);
> + data->access = ktime_get();
> +
> + return

Re: [RFC v1 PATCH 3/3] driver: update all the code that use soc_device_match

[Guenter Roeck]

On 4/18/21 9:27 PM, Alice Guo (OSS) wrote:
> From: Alice Guo 
> 
> Update all the code that use soc_device_match because add support for
> soc_device_match returning -EPROBE_DEFER.
> 
> Signed-off-by: Alice Guo 
> ---
[ ... ]
>  drivers/watchdog/renesas_wdt.c|  2 +-
>  48 files changed, 131 insertions(+), 52 deletions(-)
> 
[ ... ]
> diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c
> index 5791198960e6..fdc534dc4024 100644
> --- a/drivers/watchdog/renesas_wdt.c
> +++ b/drivers/watchdog/renesas_wdt.c
> @@ -197,7 +197,7 @@ static bool rwdt_blacklisted(struct device *dev)
>   const struct soc_device_attribute *attr;
>  
>   attr = soc_device_match(rwdt_quirks_match);
> - if (attr && setup_max_cpus > (uintptr_t)attr->data) {
> + if (!IS_ERR(attr) && attr && setup_max_cpus > (uintptr_t)attr->data) {

This is wrong. We can not make the decision below without having access
to attr. The function may wrongly return false if soc_device_match()
returns an error.

Guenter

>   dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id,
>attr->revision);
>   return true;
> 

Re: Linux 5.12-rc8

[Guenter Roeck]

On Sun, Apr 18, 2021 at 03:02:49PM -0700, Linus Torvalds wrote:
> Ok, so it's been _fairly_ calm this past week, but it hasn't been the
> kind of dead calm I would have taken to mean "no rc8 necessary".
> 
> So here we are, with an extra rc to make sure things are all settled
> down. It's not _that_ rare: this is the fifth time in the 5.x series
> we've ended up with an rc8, but I have to admit that I prefer it when
> a release doesn't end up needing that extra week.
> 
> Because let's keep it to just one extra week, ok? We have occasionally
> done rc9's too, but I really don't expect that this time around.
> 
> About half of this is once more networking, with driver and bpf
> verifier fixes standing out. Other than that it's mostly other driver
> updates (gpu, dmaengine, HID, input, nvdimm) and arch updates (mainly
> arm and arm64).
> 
> And a number of one-liner build fixes for unusual configurations.
> 
> So it's not tiny, but it's all small enough that you can easily scan
> through the shortlog below and get a fair sense of what's going on.
> 
> Let's plan on a final 5.12 release next weekend - but please do give
> it one last test to check that it is all solid. Ok?
> 

Build results:
total: 151 pass: 151 fail: 0
Qemu test results:
total: 461 pass: 461 fail: 0

Guenter

Re: [PATCH] watchdog: aspeed: fix hardware timeout calculation

[Guenter Roeck]

On Fri, Apr 16, 2021 at 08:42:49PM -0700, rentao.b...@gmail.com wrote:
> From: Tao Ren 
> 
> Fix hardware timeout calculation in aspeed_wdt_set_timeout function to
> ensure the reload value does not exceed the hardware limit.
> 
> Fixes: efa859f7d786 ("watchdog: Add Aspeed watchdog driver")
> Reported-by: Amithash Prasad 
> Signed-off-by: Tao Ren 

Reviewed-by: Guenter Roeck 

> ---
>  drivers/watchdog/aspeed_wdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 7e00960651fa..507fd815d767 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -147,7 +147,7 @@ static int aspeed_wdt_set_timeout(struct watchdog_device 
> *wdd,
>  
>   wdd->timeout = timeout;
>  
> - actual = min(timeout, wdd->max_hw_heartbeat_ms * 1000);
> + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
>  
>   writel(actual * WDT_RATE_1MHZ, wdt->base + WDT_RELOAD_VALUE);
>   writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART);
> -- 
> 2.17.1
> 

Re: [PATCH v4 1/3] riscv: Move kernel mapping outside of linear mapping

[Guenter Roeck]

On Fri, Apr 09, 2021 at 02:14:58AM -0400, Alexandre Ghiti wrote:
> This is a preparatory patch for relocatable kernel and sv48 support.
> 
> The kernel used to be linked at PAGE_OFFSET address therefore we could use
> the linear mapping for the kernel mapping. But the relocated kernel base
> address will be different from PAGE_OFFSET and since in the linear mapping,
> two different virtual addresses cannot point to the same physical address,
> the kernel mapping needs to lie outside the linear mapping so that we don't
> have to copy it at the same physical offset.
> 
> The kernel mapping is moved to the last 2GB of the address space, BPF
> is now always after the kernel and modules use the 2GB memory range right
> before the kernel, so BPF and modules regions do not overlap. KASLR
> implementation will simply have to move the kernel in the last 2GB range
> and just take care of leaving enough space for BPF.
> 
> In addition, by moving the kernel to the end of the address space, both
> sv39 and sv48 kernels will be exactly the same without needing to be
> relocated at runtime.
> 
> Suggested-by: Arnd Bergmann 
> Signed-off-by: Alexandre Ghiti 

In next-20210416, when booting a riscv32 image in qemu, this patch results in:

[0.00] Linux version 5.12.0-rc7-next-20210416 (groeck@desktop) 
(riscv32-linux-gcc (GCC) 10.3.0, GNU ld (GNU Binutils) 2.36.1) #1 SMP Fri Apr 
16 10:38:09 PDT 2021
[0.00] OF: fdt: Ignoring memory block 0x8000 - 0xa000
[0.00] Machine model: riscv-virtio,qemu
[0.00] earlycon: uart8250 at MMIO 0x1000 (options '115200')
[0.00] printk: bootconsole [uart8250] enabled
[0.00] efi: UEFI not found.
[0.00] Kernel panic - not syncing: init_resources: Failed to allocate 
160 bytes
[0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 5.12.0-rc7-next-20210416 
#1
[0.00] Hardware name: riscv-virtio,qemu (DT)
[0.00] Call Trace:
[0.00] [<80005292>] walk_stackframe+0x0/0xce
[0.00] [<809f4db8>] dump_backtrace+0x38/0x46
[0.00] [<809f4dd4>] show_stack+0xe/0x16
[0.00] [<809ff1d0>] dump_stack+0x92/0xc6
[0.00] [<809f4fee>] panic+0x10a/0x2d8
[0.00] [<80c02b24>] setup_arch+0x2a0/0x4ea
[0.00] [<80c006b0>] start_kernel+0x90/0x628
[0.00] ---[ end Kernel panic - not syncing: init_resources: Failed to 
allocate 160 bytes ]---

Reverting it fixes the problem. I understand that the version in -next is
different to this version of the patch, but I also tried v4 and it still
crashes with the same error message.

Guenter

Re: [PATCH v2] hwmon: pmbus: pxe1610: don't bail out when not all pages are active

[Guenter Roeck]

On Fri, Apr 16, 2021 at 01:29:04PM +0300, Paul Fertser wrote:
> Certain VRs might be configured to use only the first output channel and
> so the mode for the second will be 0. Handle this gracefully.
> 
> Fixes: b9fa0a3acfd8 ("hwmon: (pmbus/core) Add support for vid mode detection 
> per page bases")
> Signed-off-by: Paul Fertser 

Applied.

Thanks,
Guenter

> ---
> 
> Notes:
> Changes for v2:
>   - Use more imperative style
> 
>  drivers/hwmon/pmbus/pxe1610.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
> index da27ce34ee3f..eb4a06003b7f 100644
> --- a/drivers/hwmon/pmbus/pxe1610.c
> +++ b/drivers/hwmon/pmbus/pxe1610.c
> @@ -41,6 +41,15 @@ static int pxe1610_identify(struct i2c_client *client,
>   info->vrm_version[i] = vr13;
>   break;
>   default:
> + /*
> +  * If prior pages are available limit operation
> +  * to them
> +  */
> + if (i != 0) {
> + info->pages = i;
> + return 0;
> + }
> +
>   return -ENODEV;
>   }
>   }
> -- 
> 2.20.1
> 

Re: [PATCH v2] watchdog: aspeed: fix integer overflow in set_timeout handler

[Guenter Roeck]

On 4/15/21 7:13 PM, rentao.b...@gmail.com wrote:
> From: Tao Ren 
> 
> Fix the time comparison (timeout vs. max_hw_heartbeat_ms) in set_timeout
> handler to avoid potential integer overflow when the supplied timeout is
> greater than aspeed's maximum allowed timeout (4294 seconds).
> 

I think this is the wrong focus: What this fixes is the wrong hardware
timeout calculation. Again, I think that the wrong calculation leads to
the overflow should not be the focus of this patch, though it can of
course be mentioned.

I'll leave it up to Wim to decide if he wants to apply the patch with the
current explanation.

Thanks,
Guenter

> Fixes: efa859f7d786 ("watchdog: Add Aspeed watchdog driver")
> Reported-by: Amithash Prasad 
> Signed-off-by: Tao Ren 
> ---
>  Changes in v2:
>- do not touch "wdd->timeout": only "max_hw_heartbeat_ms * 1000" is
>  updated to "max_hw_heartbeat_ms / 1000".
> 
>  drivers/watchdog/aspeed_wdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 7e00960651fa..507fd815d767 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -147,7 +147,7 @@ static int aspeed_wdt_set_timeout(struct watchdog_device 
> *wdd,
>  
>   wdd->timeout = timeout;
>  
> - actual = min(timeout, wdd->max_hw_heartbeat_ms * 1000);
> + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
>  
>   writel(actual * WDT_RATE_1MHZ, wdt->base + WDT_RELOAD_VALUE);
>   writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART);
> 

Re: [PATCH] watchdog: aspeed: fix integer overflow in set_timeout handler

[Guenter Roeck]

On Thu, Apr 15, 2021 at 06:04:45PM -0700, Tao Ren wrote:
> On Thu, Apr 15, 2021 at 05:50:32PM -0700, Guenter Roeck wrote:
> > On 4/15/21 5:12 PM, rentao.b...@gmail.com wrote:
> > > From: Tao Ren 
> > > 
> > > Fix the time comparison (timeout vs. max_hw_heartbeat_ms) in set_timeout
> > > handler to avoid potential integer overflow when the supplied timeout is
> > > greater than aspeed's maximum allowed timeout (4294 seconds).
> > > 
> > > Fixes: efa859f7d786 ("watchdog: Add Aspeed watchdog driver")
> > > Reported-by: Amithash Prasad 
> > > Signed-off-by: Tao Ren 
> > > ---
> > >  drivers/watchdog/aspeed_wdt.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> > > index 7e00960651fa..9f77272dc906 100644
> > > --- a/drivers/watchdog/aspeed_wdt.c
> > > +++ b/drivers/watchdog/aspeed_wdt.c
> > > @@ -145,9 +145,8 @@ static int aspeed_wdt_set_timeout(struct 
> > > watchdog_device *wdd,
> > >   struct aspeed_wdt *wdt = to_aspeed_wdt(wdd);
> > >   u32 actual;
> > >  
> > > - wdd->timeout = timeout;
> > > -
> > > - actual = min(timeout, wdd->max_hw_heartbeat_ms * 1000);
> > > + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
> > > + wdd->timeout = actual;
> > >  
> > >   writel(actual * WDT_RATE_1MHZ, wdt->base + WDT_RELOAD_VALUE);
> > >   writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART);
> > > 
> > 
> > If the provided timeout is larger than the supported hardware timeout,
> > the watchdog core will ping the hardware on behalf of userspace.
> > The above code would defeat that mechanism for no good reason.
> > 
> > NACK.
> > 
> > Guenter
> 
> Thanks Guenter for Joel for the quick review!
> 
> The integer overflow happens at (actual * WDT_RATE_1MHZ). For example,
> if a user tries to set timeout to 4295 seconds, then the hardware would
> be programmed to timeout after about 32 milliseconds. I would say this
> behavior is not expected?

The fix would be

-   actual = min(timeout, wdd->max_hw_heartbeat_ms * 1000);
+   actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);

without modifying
wdd->timeout = timeout;

The real problem here is that "wdd->max_hw_heartbeat_ms * 1000"
is simply wrong. The overflow is a side effect of the wrong
calculation, and concentrating on it disguises the real problem.

Guenter

Re: [PATCH] watchdog: aspeed: fix integer overflow in set_timeout handler

[Guenter Roeck]

On 4/15/21 5:12 PM, rentao.b...@gmail.com wrote:
> From: Tao Ren 
> 
> Fix the time comparison (timeout vs. max_hw_heartbeat_ms) in set_timeout
> handler to avoid potential integer overflow when the supplied timeout is
> greater than aspeed's maximum allowed timeout (4294 seconds).
> 
> Fixes: efa859f7d786 ("watchdog: Add Aspeed watchdog driver")
> Reported-by: Amithash Prasad 
> Signed-off-by: Tao Ren 
> ---
>  drivers/watchdog/aspeed_wdt.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 7e00960651fa..9f77272dc906 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -145,9 +145,8 @@ static int aspeed_wdt_set_timeout(struct watchdog_device 
> *wdd,
>   struct aspeed_wdt *wdt = to_aspeed_wdt(wdd);
>   u32 actual;
>  
> - wdd->timeout = timeout;
> -
> - actual = min(timeout, wdd->max_hw_heartbeat_ms * 1000);
> + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
> + wdd->timeout = actual;
>  
>   writel(actual * WDT_RATE_1MHZ, wdt->base + WDT_RELOAD_VALUE);
>   writel(WDT_RESTART_MAGIC, wdt->base + WDT_RESTART);
> 

If the provided timeout is larger than the supported hardware timeout,
the watchdog core will ping the hardware on behalf of userspace.
The above code would defeat that mechanism for no good reason.

NACK.

Guenter

Re: [PATCH] hwmon: pmbus: pxe1610: don't bail out when not all pages are active

[Guenter Roeck]

On 4/15/21 6:34 AM, Paul Fertser wrote:
> Certain VRs might be configured to use only the first output channel and
> so the mode for the second will be 0. Handle this gracefully.
> 
> Fixes: b9fa0a3acfd8 ("hwmon: (pmbus/core) Add support for vid mode detection 
> per page bases")
> Signed-off-by: Paul Fertser 
> ---
>  drivers/hwmon/pmbus/pxe1610.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
> index 517584cff3de..ff6f6e05923b 100644
> --- a/drivers/hwmon/pmbus/pxe1610.c
> +++ b/drivers/hwmon/pmbus/pxe1610.c
> @@ -41,7 +41,16 @@ static int pxe1610_identify(struct i2c_client *client,
>   info->vrm_version[i] = vr13;
>   break;
>   default:
> - return -ENODEV;
> + /*
> +  * If prior pages are available limit operation
> +  * to them
> +  */
> + if (i != 0) {
> + info->pages = i;
> + return 0;
> + } else {
> + return -ENODEV;
> + }

else after return is unnecessary.

Thanks,
Guenter

Re: [PATCH V3] watchdog: mtk: support pre-timeout when the bark irq is available

[Guenter Roeck]

On 4/15/21 2:40 AM, 王擎 wrote:

> 
> Yes, as mentioned before, the behavior of WDT_MODE_IRQ_EN is use irq instead 
> of
> reset, so we must use WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN if like you said
> "the first time generating an interrupt and the second time resetting the 
> system" . 
> 
> The Dual mode is mentioned in the MTK datasheet:
> In this mode, the watchdog will be AUTO-RESTART after interrupt is triggered. 
> AP need to clear WDT_STA after receiving interrupt from TOPRGU, or system 
> reset
> will be triggered after watchdog timer expires.
> Instructions for use：
> Set wdt_en = 1'b1.
> Set dual_mode = 1'b1.
> Set wdt_irq = 1'b1.
> 
> We can use Dual mode to achieve pretimeout behavior, only in this way can we
> get more information during pretimeout processing, instead of directly 
> resetting.
> 

If so, the pretimeout would always be half of the full timeout, and the timeout
to configure would be timeout / 2. The code would have to adjust the pretimeout
time accordingly. Also, to avoid unexpected and changed behavior, the prefimeout
should by default be disabled.

Guenter

Re: [PATCH v1] usb: typec: tcpm: Fix error while calculating PPS out values

[Guenter Roeck]

On 4/14/21 10:01 PM, Badhri Jagan Sridharan wrote:
> "usb: typec: tcpm: Address incorrect values of tcpm psy for pps supply"
> introduced a regression for req_out_volt and req_op_curr calculation.
> 
> req_out_volt should consider the newly calculated max voltage instead
> of previously accepted max voltage by the port partner. Likewise,
> req_op_curr should consider the newly calculated max current instead
> of previously accepted max current by the port partner.
> 
> Fixes: e3a072022487 ("usb: typec: tcpm: Address incorrect values of tcpm psy 
> for pps supply")
> Signed-off-by: Badhri Jagan Sridharan 

Reviewed-by: Guenter Roeck 

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 1c32bdf62852..04652aa1f54e 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -3132,10 +3132,10 @@ static unsigned int tcpm_pd_select_pps_apdo(struct 
> tcpm_port *port)
>   port->pps_data.req_max_volt = min(pdo_pps_apdo_max_voltage(src),
> 
> pdo_pps_apdo_max_voltage(snk));
>   port->pps_data.req_max_curr = min_pps_apdo_current(src, snk);
> - port->pps_data.req_out_volt = min(port->pps_data.max_volt,
> -   max(port->pps_data.min_volt,
> + port->pps_data.req_out_volt = min(port->pps_data.req_max_volt,
> +   
> max(port->pps_data.req_min_volt,
> 
> port->pps_data.req_out_volt));
> - port->pps_data.req_op_curr = min(port->pps_data.max_curr,
> + port->pps_data.req_op_curr = min(port->pps_data.req_max_curr,
>port->pps_data.req_op_curr);
>   }
>  
> 

Re: [PATCH v3 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver

[Guenter Roeck]

On Fri, Apr 09, 2021 at 10:13:31AM +0700, Quan Nguyen wrote:
> This commit adds support for Ampere SMpro hwmon driver. This driver
> supports accessing various CPU sensors provided by the SMpro co-processor
> including temperature, power, voltages, and current.
> 
> Signed-off-by: Quan Nguyen 
> ---

Change log goes here. You are making it difficult to review your patches.

>  drivers/hwmon/Kconfig   |   8 +
>  drivers/hwmon/Makefile  |   1 +
>  drivers/hwmon/smpro-hwmon.c | 491 
>  3 files changed, 500 insertions(+)
>  create mode 100644 drivers/hwmon/smpro-hwmon.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 0ddc974b102e..ba4b5a911baf 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -67,6 +67,14 @@ config SENSORS_ABITUGURU3
> This driver can also be built as a module. If so, the module
> will be called abituguru3.
>  
> +config SENSORS_SMPRO
> + tristate "Ampere's Altra SMpro hardware monitoring driver"
> + depends on MFD_SMPRO
> + help
> +   If you say yes here you get support for the thermal, voltage,
> +   current and power sensors of Ampere's Altra processor family SoC
> +   with SMpro co-processor.
> +
>  config SENSORS_AD7314
>   tristate "Analog Devices AD7314 and compatibles"
>   depends on SPI
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 59e78bc212cf..b25391f9c651 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_SHT3x)   += sht3x.o
>  obj-$(CONFIG_SENSORS_SHTC1)  += shtc1.o
>  obj-$(CONFIG_SENSORS_SIS5595)+= sis5595.o
>  obj-$(CONFIG_SENSORS_SMM665) += smm665.o
> +obj-$(CONFIG_SENSORS_SMPRO)  += smpro-hwmon.o
>  obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
>  obj-$(CONFIG_SENSORS_SMSC47M1)   += smsc47m1.o
>  obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
> diff --git a/drivers/hwmon/smpro-hwmon.c b/drivers/hwmon/smpro-hwmon.c
> new file mode 100644
> index ..a3389fcbad82
> --- /dev/null
> +++ b/drivers/hwmon/smpro-hwmon.c
> @@ -0,0 +1,491 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Ampere Computing SoC's SMPro Hardware Monitoring Driver
> + *
> + * Copyright (c) 2021, Ampere Computing LLC
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Identification Registers */
> +#define MANUFACTURER_ID_REG  0x02
> +#define AMPERE_MANUFACTURER_ID   0xCD3A
> +
> +/* Logical Power Sensor Registers */
> +#define SOC_TEMP 0x00
> +#define SOC_VRD_TEMP 0x01
> +#define DIMM_VRD_TEMP0x02
> +#define CORE_VRD_TEMP0x03
> +#define CH0_DIMM_TEMP0x04
> +#define CH1_DIMM_TEMP0x05
> +#define CH2_DIMM_TEMP0x06
> +#define CH3_DIMM_TEMP0x07
> +#define CH4_DIMM_TEMP0x08
> +#define CH5_DIMM_TEMP0x09
> +#define CH6_DIMM_TEMP0x0A
> +#define CH7_DIMM_TEMP0x0B
> +#define RCA_VRD_TEMP 0x0C
> +
> +#define CORE_VRD_PWR 0x10
> +#define SOC_PWR  0x11
> +#define DIMM_VRD1_PWR0x12
> +#define DIMM_VRD2_PWR0x13
> +#define CORE_VRD_PWR_MW  0x16
> +#define SOC_PWR_MW   0x17
> +#define DIMM_VRD1_PWR_MW 0x18
> +#define DIMM_VRD2_PWR_MW 0x19
> +#define RCA_VRD_PWR  0x1A
> +#define RCA_VRD_PWR_MW   0x1B
> +
> +#define MEM_HOT_THRESHOLD0x22
> +#define SOC_VR_HOT_THRESHOLD 0x23
> +#define CORE_VRD_VOLT0x24
> +#define SOC_VRD_VOLT 0x25
> +#define DIMM_VRD1_VOLT   0x26
> +#define DIMM_VRD2_VOLT   0x27
> +#define RCA_VRD_VOLT 0x28
> +
> +#define CORE_VRD_CURR0x29
> +#define SOC_VRD_CURR 0x2A
> +#define DIMM_VRD1_CURR   0x2B
> +#define DIMM_VRD2_CURR   0x2C
> +#define RCA_VRD_CURR 0x2D
> +
> +struct smpro_hwmon {
> + struct regmap *regmap;
> + u32 offset;
> +};
> +
> +struct smpro_sensor {
> + const u8 reg;
> + const u8 reg_ext;
> + const char *label;
> +};
> +
> +static const struct smpro_sensor temperature[] = {
> + {
> + .reg = SOC_TEMP,
> + .label = "temp1 SoC"
> + },
> + {
> + .reg = SOC_VRD_TEMP,
> + .reg_ext = SOC_VR_HOT_THRESHOLD,
> + .label = "temp2 SoC VRD"
> + },
> + {
> + .reg = DIMM_VRD_TEMP,
> + .label = "temp3 DIMM VRD"
> + },
> + {
> + .reg = CORE_VRD_TEMP,
> + .label = "temp4 CORE VRD"
> + },
> + {
> + .reg = CH0_DIMM_TEMP,
> + .reg_ext = MEM_HOT_THRESHOLD,
> + .label = "temp5 CH0 DIMM"
> + },
> + {
> + .reg = CH1_DIMM_TEMP,
> + 

Re: [PATCH v3 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support

[Guenter Roeck]

On Fri, Apr 09, 2021 at 10:13:30AM +0700, Quan Nguyen wrote:
> Adds an MFD driver for SMpro found on the Mt.Jade hardware reference
> platform with Ampere's Altra processor family.
> 
> Signed-off-by: Quan Nguyen 
> Reported-by: kernel test robot 
> ---
>  drivers/mfd/Kconfig  | 10 ++
>  drivers/mfd/simple-mfd-i2c.c |  6 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index d07e8cf93286..f7a6460f7aa0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -77,6 +77,16 @@ config MFD_AS3711
>   help
> Support for the AS3711 PMIC from AMS
>  
> +config MFD_SMPRO
> + tristate "Ampere Computing MFD SMpro core driver"
> + select MFD_SIMPLE_MFD_I2C

This is missing "depends on I2C".

> + help
> +   Say yes here to enable SMpro driver support for Ampere's Altra
> +   processor family.
> +
> +   Ampere's Altra SMpro exposes an I2C regmap interface that can
> +   be accessed by child devices.
> +
>  config MFD_AS3722
>   tristate "ams AS3722 Power Management IC"
>   select MFD_CORE
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index 87f684cff9a1..9a44655f5592 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -21,6 +21,11 @@ static const struct regmap_config simple_regmap_config = {
>   .val_bits = 8,
>  };
>  
> +static const struct regmap_config simple_word_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> +};
> +
>  static int simple_mfd_i2c_probe(struct i2c_client *i2c)
>  {
>   const struct regmap_config *config;
> @@ -39,6 +44,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c)
>  
>  static const struct of_device_id simple_mfd_i2c_of_match[] = {
>   { .compatible = "kontron,sl28cpld" },
> + { .compatible = "ampere,smpro", .data = _word_regmap_config },
>   {}
>  };
>  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

Re: [PATCH V3] watchdog: mtk: support pre-timeout when the bark irq is available

[Guenter Roeck]

On 4/14/21 4:48 AM, Wang Qing wrote:
> Use the bark interrupt as the pretimeout notifier if available.
> 
> By default, the pretimeout notification shall occur one second earlier
> than the timeout.
> 
> V2:
> - panic() by default if WATCHDOG_PRETIMEOUT_GOV is not enabled.
> 
> V3:
> - Modify the pretimeout behavior, manually reset after the pretimeout
> - is processed and wait until timeout.
> 
> Signed-off-by: Wang Qing 
> ---
>  drivers/watchdog/mtk_wdt.c | 62 
> ++
>  1 file changed, 57 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 97ca993..7bef1e3
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define WDT_MAX_TIMEOUT  31
>  #define WDT_MIN_TIMEOUT  1
> @@ -234,18 +235,46 @@ static int mtk_wdt_start(struct watchdog_device 
> *wdt_dev)
>   void __iomem *wdt_base = mtk_wdt->wdt_base;
>   int ret;
>  
> - ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> + ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - 
> wdt_dev->pretimeout);
>   if (ret < 0)
>   return ret;
>  
>   reg = ioread32(wdt_base + WDT_MODE);
> - reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
> + reg &= ~WDT_MODE_IRQ_EN;
> + if (wdt_dev->pretimeout)
> + reg |= WDT_MODE_IRQ_EN;
> + else
> + reg &= ~WDT_MODE_IRQ_EN;
>   reg |= (WDT_MODE_EN | WDT_MODE_KEY);
>   iowrite32(reg, wdt_base + WDT_MODE);
>  
>   return 0;
>  }
>  
> +static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd,
> +unsigned int timeout)
> +{
> + wdd->pretimeout = timeout;
> + return mtk_wdt_start(wdd);

The watchdog is not necessarily active here.

> +}
> +
> +static irqreturn_t mtk_wdt_isr(int irq, void *arg)
> +{
> + struct watchdog_device *wdd = arg;
> + struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd);
> + void __iomem *wdt_base = mtk_wdt->wdt_base;
> +
> + watchdog_notify_pretimeout(wdd);
> + /*
> +  * Guaranteed to be reset when the timeout
> +  * expires under any situations
> +  */
> + mdelay(1000*wdd->pretimeout);

That is not how this is supposed to work. The idea with a pretimeout is that the
real watchdog reset will happen under all circumstances, and that executing
the pretimeout (and changing some hardware registers) is not a prerequisite
for the real timeout to happen. After all, the system could be stuck hard, with
interrupts disabled.

On top of that, just sleeping here while waiting for the real timeout and
then resetting the system isn't the idea either. On a single core system this
will just hang. On a multi-core system, who knows if userspace managed to ping
the watchdog in the meantime.

Unless there is a means to trigger the watchdog twice, without intervention,
the first time generating an interrupt and the second time resetting the system,
there is no way for this to work. I don't see how this chip really supports
pretimeout. It seems that it supports either a hard reset or generating an
interrupt on watchdog timeout, and there is only a single timeout.

If you have a use case for generating an interrupt and resetting the system via
software (ie panic) _instead_ of having it generate a hard reset, please feel
free to submit a patch along that line, together with a description of its use
case.

Thanks,
Guenter

> + writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST);
> +
> + return IRQ_HANDLED;
> +}
> +
>  static const struct watchdog_info mtk_wdt_info = {
>   .identity   = DRV_NAME,
>   .options= WDIOF_SETTIMEOUT |
> @@ -253,12 +282,21 @@ static const struct watchdog_info mtk_wdt_info = {
> WDIOF_MAGICCLOSE,
>  };
>  
> +static const struct watchdog_info mtk_wdt_pt_info = {
> + .identity   = DRV_NAME,
> + .options= WDIOF_SETTIMEOUT |
> +   WDIOF_PRETIMEOUT |
> +   WDIOF_KEEPALIVEPING |
> +   WDIOF_MAGICCLOSE,
> +};
> +
>  static const struct watchdog_ops mtk_wdt_ops = {
>   .owner  = THIS_MODULE,
>   .start  = mtk_wdt_start,
>   .stop   = mtk_wdt_stop,
>   .ping   = mtk_wdt_ping,
>   .set_timeout= mtk_wdt_set_timeout,
> + .set_pretimeout = mtk_wdt_set_pretimeout,
>   .restart= mtk_wdt_restart,
>  };
>  
> @@ -267,7 +305,7 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   struct device *dev = >dev;
>   struct mtk_wdt_dev *mtk_wdt;
>   const struct mtk_wdt_data *wdt_data;
> - int err;
> + int err, irq;
>  
>   mtk_wdt = devm_kzalloc(dev, sizeof(*mtk_wdt), GFP_KERNEL);
>   if (!mtk_wdt)
> @@ -279,7 +317,22 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   if (IS_ERR(mtk_wdt->wdt_base))
>

Re: [PATCH V2] watchdog: mtk: support pre-timeout when the bark irq is available

[Guenter Roeck]

On 4/14/21 12:28 AM, Wang Qing wrote:
> Use the bark interrupt as the pretimeout notifier if available.
> 
> By default, the pretimeout notification shall occur one second earlier
> than the timeout.
> 
> V2:
> - panic() by default if WATCHDOG_PRETIMEOUT_GOV is not enabled
> 
> Signed-off-by: Wang Qing 
> ---
>  drivers/watchdog/mtk_wdt.c | 57 
> ++
>  1 file changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 97ca993..b0ec933
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define WDT_MAX_TIMEOUT  31
>  #define WDT_MIN_TIMEOUT  1
> @@ -234,18 +235,41 @@ static int mtk_wdt_start(struct watchdog_device 
> *wdt_dev)
>   void __iomem *wdt_base = mtk_wdt->wdt_base;
>   int ret;
>  
> - ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> + ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - 
> wdt_dev->pretimeout);
>   if (ret < 0)
>   return ret;
>  
>   reg = ioread32(wdt_base + WDT_MODE);
> - reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
> + reg &= ~WDT_MODE_IRQ_EN;
> + if (wdt_dev->pretimeout)
> + reg |= WDT_MODE_IRQ_EN;
> + else
> + reg &= ~WDT_MODE_IRQ_EN;
>   reg |= (WDT_MODE_EN | WDT_MODE_KEY);
>   iowrite32(reg, wdt_base + WDT_MODE);
> 


You said previously that the pretimeout would prevent the real timeout / reset
from happening, and the driver has no provision to set the real timeout.
That makes this a no-go.

>   return 0;
>  }
>  
> +static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd,
> +unsigned int timeout)
> +{
> + wdd->pretimeout = timeout;
> + return mtk_wdt_start(wdd);
> +}
> +
> +static irqreturn_t mtk_wdt_isr(int irq, void *arg)
> +{
> + struct watchdog_device *wdd = arg;
> +if (IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV))
> + watchdog_notify_pretimeout(wdd);
> +else
> + //panic by decault instead of WDT_SWRST;
> + panic("MTk Watchdog bark!\n");
> +
The idea behind the pretimeout governor is that it can be disabled
by setting it to "none", and if pretimeout governors are disabled
it is supposed to be a no-op. The above bypasses this mechanism
and is not acceptable.

This suggests that the real timeout indeed doesn't happen when
WDT_MODE_IRQ_EN is set. Again, this is unacceptable.

On a side note, formatting is way off here.

Guenter

> + return IRQ_HANDLED;
> +}
> +
>  static const struct watchdog_info mtk_wdt_info = {
>   .identity   = DRV_NAME,
>   .options= WDIOF_SETTIMEOUT |
> @@ -253,12 +277,21 @@ static const struct watchdog_info mtk_wdt_info = {
> WDIOF_MAGICCLOSE,
>  };
>  
> +static const struct watchdog_info mtk_wdt_pt_info = {
> + .identity   = DRV_NAME,
> + .options= WDIOF_SETTIMEOUT |
> +   WDIOF_PRETIMEOUT |
> +   WDIOF_KEEPALIVEPING |
> +   WDIOF_MAGICCLOSE,
> +};
> +
>  static const struct watchdog_ops mtk_wdt_ops = {
>   .owner  = THIS_MODULE,
>   .start  = mtk_wdt_start,
>   .stop   = mtk_wdt_stop,
>   .ping   = mtk_wdt_ping,
>   .set_timeout= mtk_wdt_set_timeout,
> + .set_pretimeout = mtk_wdt_set_pretimeout,
>   .restart= mtk_wdt_restart,
>  };
>  
> @@ -267,7 +300,7 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   struct device *dev = >dev;
>   struct mtk_wdt_dev *mtk_wdt;
>   const struct mtk_wdt_data *wdt_data;
> - int err;
> + int err, irq;
>  
>   mtk_wdt = devm_kzalloc(dev, sizeof(*mtk_wdt), GFP_KERNEL);
>   if (!mtk_wdt)
> @@ -279,7 +312,22 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   if (IS_ERR(mtk_wdt->wdt_base))
>   return PTR_ERR(mtk_wdt->wdt_base);
>  
> - mtk_wdt->wdt_dev.info = _wdt_info;
> + irq = platform_get_irq(pdev, 0);
> + if (irq > 0) {
> + err = devm_request_irq(>dev, irq, mtk_wdt_isr, 0, 
> "wdt_bark",
> + 
> _wdt->wdt_dev);
> + if (err)
> + return err;
> +
> + mtk_wdt->wdt_dev.info = _wdt_pt_info;
> + mtk_wdt->wdt_dev.pretimeout = 1;
> + } else {
> + if (irq == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + mtk_wdt->wdt_dev.info = _wdt_info;
> + }
> +
>   mtk_wdt->wdt_dev.ops = _wdt_ops;
>   mtk_wdt->wdt_dev.timeout = WDT_MAX_TIMEOUT;
>   mtk_wdt->wdt_dev.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT * 1000;
> @@ -360,7 +408,6 @@ static struct platform_driver mtk_wdt_driver = {
>  };
>  
>  module_platform_driver(mtk_wdt_driver);
> -
>  module_param(timeout, uint, 0);
>  

Re: [PATCH v2 2/2] hwmon: intel-m10-bmc-hwmon: add sensor support of Intel D5005 card

[Guenter Roeck]

On Tue, Apr 13, 2021 at 03:58:35PM -0700, matthew.gerl...@linux.intel.com wrote:
> From: Matthew Gerlach 
> 
> Like the Intel N3000 card, the Intel D5005 has a MAX10 based
> BMC.  This commit adds support for the D5005 sensors that are
> monitored by the MAX10 BMC.
> 
> Signed-off-by: Matthew Gerlach 
> Signed-off-by: Russ Weight 
> Acked-by: Lee Jones 

Applied to hwmon-next.

Thanks,
Guenter

> ---
> v2: change variable name from m10bmc_bmc_subdevs to m10bmc_d5005_subdevs
> added Acked-by: Lee Jones
> ---
>  drivers/hwmon/intel-m10-bmc-hwmon.c | 122 
> 
>  drivers/mfd/intel-m10-bmc.c |  10 +++
>  2 files changed, 132 insertions(+)
> 
> diff --git a/drivers/hwmon/intel-m10-bmc-hwmon.c 
> b/drivers/hwmon/intel-m10-bmc-hwmon.c
> index 17d5e6b..bd7ed2e 100644
> --- a/drivers/hwmon/intel-m10-bmc-hwmon.c
> +++ b/drivers/hwmon/intel-m10-bmc-hwmon.c
> @@ -99,6 +99,50 @@ struct m10bmc_hwmon {
>   NULL
>  };
>  
> +static const struct m10bmc_sdata d5005bmc_temp_tbl[] = {
> + { 0x100, 0x104, 0x108, 0x10c, 0x0, 500, "Board Inlet Air Temperature" },
> + { 0x110, 0x114, 0x118, 0x0, 0x0, 500, "FPGA Core Temperature" },
> + { 0x11c, 0x120, 0x124, 0x128, 0x0, 500, "Board Exhaust Air Temperature" 
> },
> + { 0x12c, 0x130, 0x134, 0x0, 0x0, 500, "FPGA Transceiver Temperature" },
> + { 0x138, 0x13c, 0x140, 0x144, 0x0, 500, "RDIMM0 Temperature" },
> + { 0x148, 0x14c, 0x150, 0x154, 0x0, 500, "RDIMM1 Temperature" },
> + { 0x158, 0x15c, 0x160, 0x164, 0x0, 500, "RDIMM2 Temperature" },
> + { 0x168, 0x16c, 0x170, 0x174, 0x0, 500, "RDIMM3 Temperature" },
> + { 0x178, 0x17c, 0x180, 0x0, 0x0, 500, "QSFP0 Temperature" },
> + { 0x188, 0x18c, 0x190, 0x0, 0x0, 500, "QSFP1 Temperature" },
> + { 0x1a0, 0x1a4, 0x1a8, 0x0, 0x0, 500, "3.3v Temperature" },
> + { 0x1bc, 0x1c0, 0x1c4, 0x0, 0x0, 500, "VCCERAM Temperature" },
> + { 0x1d8, 0x1dc, 0x1e0, 0x0, 0x0, 500, "VCCR Temperature" },
> + { 0x1f4, 0x1f8, 0x1fc, 0x0, 0x0, 500, "VCCT Temperature" },
> + { 0x210, 0x214, 0x218, 0x0, 0x0, 500, "1.8v Temperature" },
> + { 0x22c, 0x230, 0x234, 0x0, 0x0, 500, "12v Backplane Temperature" },
> + { 0x248, 0x24c, 0x250, 0x0, 0x0, 500, "12v AUX Temperature" },
> +};
> +
> +static const struct m10bmc_sdata d5005bmc_in_tbl[] = {
> + { 0x184, 0x0, 0x0, 0x0, 0x0, 1, "QSFP0 Supply Voltage" },
> + { 0x194, 0x0, 0x0, 0x0, 0x0, 1, "QSFP1 Supply Voltage" },
> + { 0x198, 0x0, 0x0, 0x0, 0x0, 1, "FPGA Core Voltage" },
> + { 0x1ac, 0x1b0, 0x1b4, 0x0, 0x0, 1, "3.3v Voltage" },
> + { 0x1c8, 0x1cc, 0x1d0, 0x0, 0x0, 1, "VCCERAM Voltage" },
> + { 0x1e4, 0x1e8, 0x1ec, 0x0, 0x0, 1, "VCCR Voltage" },
> + { 0x200, 0x204, 0x208, 0x0, 0x0, 1, "VCCT Voltage" },
> + { 0x21c, 0x220, 0x224, 0x0, 0x0, 1, "1.8v Voltage" },
> + { 0x238, 0x0, 0x0, 0x0, 0x23c, 1, "12v Backplane Voltage" },
> + { 0x254, 0x0, 0x0, 0x0, 0x258, 1, "12v AUX Voltage" },
> +};
> +
> +static const struct m10bmc_sdata d5005bmc_curr_tbl[] = {
> + { 0x19c, 0x0, 0x0, 0x0, 0x0, 1, "FPGA Core Current" },
> + { 0x1b8, 0x0, 0x0, 0x0, 0x0, 1, "3.3v Current" },
> + { 0x1d4, 0x0, 0x0, 0x0, 0x0, 1, "VCCERAM Current" },
> + { 0x1f0, 0x0, 0x0, 0x0, 0x0, 1, "VCCR Current" },
> + { 0x20c, 0x0, 0x0, 0x0, 0x0, 1, "VCCT Current" },
> + { 0x228, 0x0, 0x0, 0x0, 0x0, 1, "1.8v Current" },
> + { 0x240, 0x244, 0x0, 0x0, 0x0, 1, "12v Backplane Current" },
> + { 0x25c, 0x260, 0x0, 0x0, 0x0, 1, "12v AUX Current" },
> +};
> +
>  static const struct m10bmc_hwmon_board_data n3000bmc_hwmon_bdata = {
>   .tables = {
>   [hwmon_temp] = n3000bmc_temp_tbl,
> @@ -110,6 +154,80 @@ struct m10bmc_hwmon {
>   .hinfo = n3000bmc_hinfo,
>  };
>  
> +static const struct hwmon_channel_info *d5005bmc_hinfo[] = {
> + HWMON_CHANNEL_INFO(temp,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_CRIT |
> +HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_CRIT |
> +HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_LABEL,
> +HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST 

Re: [PATCH] hwmon: (nct6683) remove useless function

[Guenter Roeck]

On Tue, Apr 13, 2021 at 02:02:50PM +0800, Jiapeng Chong wrote:
> Fix the following clang warning:
> 
> drivers/hwmon/nct6683.c:491:19: warning: unused function 'in_to_reg'
> [-Wunused-function].
> 
> Reported-by: Abaci Robot 
> Signed-off-by: Jiapeng Chong 

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/nct6683.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/hwmon/nct6683.c b/drivers/hwmon/nct6683.c
> index a23047a..b886cf0 100644
> --- a/drivers/hwmon/nct6683.c
> +++ b/drivers/hwmon/nct6683.c
> @@ -488,17 +488,6 @@ static inline long in_from_reg(u16 reg, u8 src)
>   return reg * scale;
>  }
>  
> -static inline u16 in_to_reg(u32 val, u8 src)
> -{
> - int scale = 16;
> -
> - if (src == MON_SRC_VCC || src == MON_SRC_VSB || src == MON_SRC_AVSB ||
> - src == MON_SRC_VBAT)
> - scale <<= 1;
> -
> - return clamp_val(DIV_ROUND_CLOSEST(val, scale), 0, 127);
> -}
> -
>  static u16 nct6683_read(struct nct6683_data *data, u16 reg)
>  {
>   int res;

Re: [PATCH] watchdog: it87_wdt: remove useless function

[Guenter Roeck]

On 4/13/21 2:34 AM, Jiapeng Chong wrote:
> Fix the following clang warning:
> 
> drivers/watchdog/it87_wdt.c:155:20: warning: unused function
> 'superio_outw' [-Wunused-function].
> 
> Reported-by: Abaci Robot 
> Signed-off-by: Jiapeng Chong 

Reviewed-by: Guenter Roeck 

> ---
>  drivers/watchdog/it87_wdt.c | 8 
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
> index 2b48318..bb11229 100644
> --- a/drivers/watchdog/it87_wdt.c
> +++ b/drivers/watchdog/it87_wdt.c
> @@ -152,14 +152,6 @@ static inline int superio_inw(int reg)
>   return val;
>  }
>  
> -static inline void superio_outw(int val, int reg)
> -{
> - outb(reg++, REG);
> - outb(val >> 8, VAL);
> - outb(reg, REG);
> - outb(val, VAL);
> -}
> -
>  /* Internal function, should be called after superio_select(GPIO) */
>  static void _wdt_update_timeout(unsigned int t)
>  {
> 

Re: [PATCH v4] hwmon: Add driver for fsp-3y PSUs and PDUs

[Guenter Roeck]

On 4/13/21 3:42 AM, Václav Kubernát wrote:
> This patch adds support for these devices:
> - YH-5151E - the PDU
> - YM-2151E - the PSU
> 
> The device datasheet says that the devices support PMBus 1.2, but in my
> testing, a lot of the commands aren't supported and if they are, they
> sometimes behave strangely or inconsistently. For example, writes to the
> PAGE command requires using PEC, otherwise the write won't work and the
> page won't switch, even though, the standard says that PEC is opiotnal.

optional

> On the other hand, writes the SMBALERT don't require PEC. Because of

s/writes the/writes to/ ?

> this, the driver is mostly reverse engineered with the help of a tool
> called pmbus_peek written by David Brownell (and later adopted by my
> colleague Jan Kundrát).
> 
> The device also has some sort of a timing issue when switching pages,
> which is explained further in the code.
> 
> Because of this, the driver support is limited. It exposes only the
> values, that have been tested to work correctly.
> 

You might want to add those details into the driver code, below the
copyright line. It would be more valuable there than in the commit log.

> Signed-off-by: Václav Kubernát 
> ---
>  Documentation/hwmon/fsp-3y.rst |  26 

Needs to be added to index.rst.

>  drivers/hwmon/pmbus/Kconfig|  10 ++
>  drivers/hwmon/pmbus/Makefile   |   1 +
>  drivers/hwmon/pmbus/fsp-3y.c   | 239 +
>  4 files changed, 276 insertions(+)
>  create mode 100644 Documentation/hwmon/fsp-3y.rst
>  create mode 100644 drivers/hwmon/pmbus/fsp-3y.c
> 
> diff --git a/Documentation/hwmon/fsp-3y.rst b/Documentation/hwmon/fsp-3y.rst
> new file mode 100644
> index ..68a547021846
> --- /dev/null
> +++ b/Documentation/hwmon/fsp-3y.rst
> @@ -0,0 +1,26 @@
> +Kernel driver fsp3y
> +==
> +Supported devices:
> +  * 3Y POWER YH-5151E
> +  * 3Y POWER YM-2151E
> +
> +Author: Václav Kubernát 
> +
> +Description
> +---
> +This driver implements limited support for two 3Y POWER devices.
> +
> +Sysfs entries
> +-
> +in1_inputinput voltage
> +in2_input12V output voltage
> +in3_input5V output voltage
> +curr1_input  input current
> +curr2_input  12V output current
> +curr3_input  5V output current
> +fan1_input   fan rpm
> +temp1_input  temperature 1
> +temp2_input  temperature 2
> +temp3_input  temperature 3
> +power1_input input power
> +power2_input output power
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 03606d4298a4..9d12d446396c 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -56,6 +56,16 @@ config SENSORS_BEL_PFE
> This driver can also be built as a module. If so, the module will
> be called bel-pfe.
>  
> +config SENSORS_FSP_3Y
> + tristate "FSP/3Y-Power power supplies"
> + help
> +   If you say yes here you get hardware monitoring support for
> +   FSP/3Y-Power hot-swap power supplies.
> +   Supported models: YH-5151E, YM-2151E
> +
> +   This driver can also be built as a module. If so, the module will
> +   be called fsp-3y.
> +
>  config SENSORS_IBM_CFFPS
>   tristate "IBM Common Form Factor Power Supply"
>   depends on LEDS_CLASS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 6a4ba0fdc1db..bfe218ad898f 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS)   += pmbus.o
>  obj-$(CONFIG_SENSORS_ADM1266)+= adm1266.o
>  obj-$(CONFIG_SENSORS_ADM1275)+= adm1275.o
>  obj-$(CONFIG_SENSORS_BEL_PFE)+= bel-pfe.o
> +obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
>  obj-$(CONFIG_SENSORS_IBM_CFFPS)  += ibm-cffps.o
>  obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
>  obj-$(CONFIG_SENSORS_IR35221)+= ir35221.o
> diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c
> new file mode 100644
> index ..2185ab119fd2
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/fsp-3y.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for FSP 3Y-Power PSUs
> + *
> + * Copyright (c) 2021 Václav Kubernát, CESNET
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "pmbus.h"
> +
> +#define YM2151_PAGE_12V_LOG  0x00
> +#define YM2151_PAGE_12V_REAL 0x00
> +#define YM2151_PAGE_5VSB_LOG 0x01
> +#define YM2151_PAGE_5VSB_REAL0x20
> +#define YH5151E_PAGE_12V_LOG 0x00
> +#define YH5151E_PAGE_12V_REAL0x00
> +#define YH5151E_PAGE_5V_LOG  0x01
> +#define YH5151E_PAGE_5V_REAL 0x10
> +#define YH5151E_PAGE_3V3_LOG 0x02
> +#define YH5151E_PAGE_3V3_REAL0x11
> +
> +enum chips {
> + ym2151e,
> + yh5151e
> +};
> +
> +struct fsp3y_data {
> + struct pmbus_driver_info info;
> + int chip;
> + 

Re: [PATCH 5.11 000/210] 5.11.14-rc1 review

[Guenter Roeck]

On Mon, Apr 12, 2021 at 10:38:25AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.11.14 release.
> There are 210 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 14 Apr 2021 08:39:44 +.
> Anything received after that time might be too late.
> 

Build results:
total: 155 pass: 155 fail: 0
Qemu test results:
total: 460 pass: 459 fail: 1
Failed tests:
sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs

udhcpc fails to get an IP address over virtio-net. I reported the same
problem against mainline. This is a spurious problem; the test succeeds
in roughly every other test. It is unknown at this time if the problem
is the patch introducing the problem (commit 0f6925b3e8da ("virtio_net:
Do not pull payload in skb->head")), the sh4 kernel code, qemu, or the
sh4 compiler (though I tried several compiler versions).

I see that this patch is now in pretty much all kernels, so I may report
this on and off until the underlying problem has been found and fixed.
Until then, I guess we'll have to live with it.

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 5.10 000/188] 5.10.30-rc1 review

[Guenter Roeck]

On Mon, Apr 12, 2021 at 10:38:34AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.10.30 release.
> There are 188 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 14 Apr 2021 08:39:44 +.
> Anything received after that time might be too late.
> 

Build results:
total: 156 pass: 156 fail: 0
Qemu test results:
total: 454 pass: 454 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 5.4 000/111] 5.4.112-rc1 review

[Guenter Roeck]

On Mon, Apr 12, 2021 at 10:39:38AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.4.112 release.
> There are 111 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 14 Apr 2021 08:39:44 +.
> Anything received after that time might be too late.
> 

Build results:
total: 157 pass: 157 fail: 0
Qemu test results:
total: 432 pass: 432 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 4.19 00/66] 4.19.187-rc1 review

[Guenter Roeck]

On Mon, Apr 12, 2021 at 10:40:06AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.19.187 release.
> There are 66 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 14 Apr 2021 08:39:44 +.
> Anything received after that time might be too late.
> 

Build results:
total: 155 pass: 155 fail: 0
Qemu test results:
total: 422 pass: 422 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: Linux 5.12-rc7

[Guenter Roeck]

On 4/12/21 10:38 AM, Eric Dumazet wrote:
[ ... ]

> Yes, I think this is the real issue here. This smells like some memory
> corruption.
> 
> In my traces, packet is correctly received in AF_PACKET queue.
> 
> I have checked the skb is well formed.
> 
> But the user space seems to never call poll() and recvmsg() on this
> af_packet socket.
> 

After sprinkling the kernel with debug messages:

424   00:01:33.674181 sendto(6, 
"E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\00148\346\1\1\6\0\246\336\333\v\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\
424   00:01:33.693873 close(6)  = 0
424   00:01:33.694652 fcntl64(5, F_SETFD, FD_CLOEXEC) = 0
424   00:01:33.695213 clock_gettime64(CLOCK_MONOTONIC, 0x7be18a18) = -1 EFAULT 
(Bad address)
424   00:01:33.695889 write(2, "udhcpc: clock_gettime(MONOTONIC) failed\n", 40) 
= -1 EFAULT (Bad address)
424   00:01:33.697311 exit_group(1) = ?
424   00:01:33.698346 +++ exited with 1 +++

I only see that after adding debug messages in the kernel, so I guess there 
must be
a heisenbug somehere.

Anyway, indeed, I see (another kernel debug message):

__do_sys_clock_gettime: Returning -EFAULT on address 0x7bacc9a8

So udhcpc doesn't even try to read the reply because it crashes after sendto()
when trying to read the current time. Unless I am missing something, that means
that the problem happens somewhere on the send side.

To make things even more interesting, it looks like the failing system call
isn't always clock_gettime().

Guenter

Re: Linux 5.12-rc7

[Guenter Roeck]

On 4/12/21 9:31 AM, Eric Dumazet wrote:
> On Mon, Apr 12, 2021 at 6:28 PM Linus Torvalds
>  wrote:
>>
>> On Sun, Apr 11, 2021 at 10:14 PM Guenter Roeck  wrote:
>>>
>>> Qemu test results:
>>> total: 460 pass: 459 fail: 1
>>> Failed tests:
>>> sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs
>>>
>>> The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull 
>>> payload in
>>> skb->head"). It is a spurious problem - the test passes roughly every other
>>> time. When the failure is seen, udhcpc fails to get an IP address and aborts
>>> with SIGTERM. So far I have only seen this with the "sh" architecture.
>>
>> Hmm. Let's add in some more of the people involved in that commit, and
>> also netdev.
>>
>> Nothing in there looks like it should have any interaction with
>> architecture, so that "it happens on sh" sounds odd, but maybe it's
>> some particular interaction with the qemu environment.
> 
> Yes, maybe.
> 
> I spent few hours on this, and suspect a buggy memcpy() implementation
> on SH, but this was not conclusive.
> 

I replaced all memcpy() calls in skbuff.h with calls to

static inline void __my_memcpy(unsigned char *to, const unsigned char *from,
   unsigned int len)
{
   while (len--)
   *to++ = *from++;
}

That made no difference, so unless you have some other memcpy() in mind that
seems to be unlikely.

> By pulling one extra byte, the problem goes away.
> 
> Strange thing is that the udhcpc process does not go past sendto().
> 

I have been trying to debug that one. Unfortunately gdb doesn't work with sh,
so I can't use it to debug the problem. I'll spend some more time on this today.

Thanks,
Guenter

Re: [PATCH v3 3/4] watchdog: simatic-ipc-wdt: add new driver for Siemens Industrial PCs

[Guenter Roeck]

On 4/12/21 8:35 AM, Henning Schild wrote:
> Am Thu, 1 Apr 2021 18:15:41 +0200
> schrieb "Enrico Weigelt, metux IT consult" :
> 
>> On 29.03.21 19:49, Henning Schild wrote:
>>
>> Hi,
>>
>>> This driver adds initial support for several devices from Siemens.
>>> It is based on a platform driver introduced in an earlier commit.  
>>
>> Where does the wdt actually come from ?
>>
>> Is it in the SoC ? (which SoC exactly). SoC-builtin wdt is a pretty 
>> usual case.
>>
>> Or some external chip ?
>>
>> The code smells a bit like two entirely different wdt's that just have
>> some similarities. If that's the case, I'd rather split it into two
>> separate drivers and let the parent driver (board file) instantiate
>> the correct one.
> 
> In fact they are the same watchdog device. The only difference is the
> "secondary enable" which controls whether the watchdog causes a reboot
> or just raises an alarm. The alarm feature is not even implemented in
> the given driver, we just enable that secondary enable regardless.
> 

Confusing statement; I can't parse "we just enable that secondary enable
regardless". What secondary enable do you enable ?

The code says "set safe_en_n so we are not just WDIOF_ALARMONLY", which
suggests that it disables the alarm feature, and does make sense.

> In one range of devices (227E) that second enable is part of a
> pio-based control register. On the other range (427E) it unfortunately
> is a P2SB gpio, which gets us right into the discussion we have around
> the LEDs.
> With that i have my doubts that two drivers would be the way to go,
> most likely not. 
> 

Reading the code again, I agree. Still, you'll need to sort out how
to determine if the watchdog or the LED driver should be enabled,
and how to access the gpio port. The GPIO pin detection and use
for 427E is a bit awkward.

Thanks,
Guenter

> Only that i have no clue which pinctrl driver should be used here. My
> guess is "sunrisepoint" because the CPUs are "SkyLake" i.e. i5-6442EQ,
> i3-6102E
> And "grep INT344B /sys/firmware/acpi/tables/DSDT" matches. I booted a
> kernel patched with the series from Andy but the "pinctrl-sunrisepoint"
> does not seem to even claim the memory. Still trying to understand how
> to make use of these pinctrl drivers they are in place but i lack
> example users (drivers). If they should be available in sysfs, i might
> be looking at the wrong place ... /sys/class/gpio/ does not list
> anything
> 
> regards,
> Henning
> 
> 
> 
>>
>> --mtx
>>
> 

Re: [PATCH] ASoC: fsl_sai: Don't use devm_regmap_init_mmio_clk

[Guenter Roeck]

On 4/12/21 3:37 AM, Shengjiu Wang wrote:
[ ... ]
> The SAI module is not supported in QEMU, so the access of the register
> failed.
> 
> you can add bypass the access in QEMU, for example:
> 
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index e0128d7316..62f7bd92a4 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -534,6 +534,10 @@ static void fsl_imx6ul_realize(DeviceState *dev,
> Error **errp)
>   */
>  create_unimplemented_device("sdma", FSL_IMX6UL_SDMA_ADDR, 0x4000);
> 
> +create_unimplemented_device("sai1", 0x02028000, 0x4000);
> +create_unimplemented_device("sai2", 0x0202c000, 0x4000);
> +create_unimplemented_device("sai3", 0x0203, 0x4000);
> 
Ah, yes, that takes care of the problem.

Thanks, and sorry for the noise.

Guenter

Re: [PATCH v1] watchdog: add new parameter to start the watchdog on module insertion

[Guenter Roeck]

On 4/12/21 1:29 AM, Flavio Suligoi wrote:
> Hi Guenter,
> 
> ...
> 
>> On 4/9/21 2:34 AM, Flavio Suligoi wrote:
>>> The new parameter "start_enabled" starts the watchdog at the same time
>>> of the module insertion.
>>> This feature is very useful in embedded systems, to avoid cases where
>>> the system hangs before reaching userspace.
>>>
>>> This function can be also enabled in the kernel config, so can be
>>> used when the watchdog driver is build as built-in.
>>>
>>> This parameter involves the "core" section of the watchdog driver;
>>> in this way it is common for all the watchdog hardware implementations.
>>>
>>> Note: to use only for watchdog drivers which doesn't support this
>>>   parameter by itself.
>>>
>>> Signed-off-by: Flavio Suligoi 
>>> ---
>>>  Documentation/watchdog/watchdog-parameters.rst |  5 +
>>>  drivers/watchdog/Kconfig   | 14 ++
>>>  drivers/watchdog/watchdog_core.c   | 12 
>>>  3 files changed, 31 insertions(+)
>>>
>>> diff --git a/Documentation/watchdog/watchdog-parameters.rst
>> b/Documentation/watchdog/watchdog-parameters.rst
>>> index 223c99361a30..623fd064df91 100644
>>> --- a/Documentation/watchdog/watchdog-parameters.rst
>>> +++ b/Documentation/watchdog/watchdog-parameters.rst
>>> @@ -21,6 +21,11 @@ watchdog core:
>>> timeout. Setting this to a non-zero value can be useful to ensure that
>>> either userspace comes up properly, or the board gets reset and allows
>>> fallback logic in the bootloader to try something else.
>>> +start_enabled:
>>> +   Watchdog is started on module insertion. This option can be also
>>> +   selected by kernel config (default=kernel config parameter).
>>> +   Use only for watchdog drivers which doesn't support this parameter
>>> +   by itself.
>>
>> Why ?
> 
> There are two drivers with an analogous feature (pnx833x_wdt and
> omap_wdt) and it is important not to enable the watchdog twice.
> 
Why ?

> Ok, I can substitute the sentence: " Use only for watchdog drivers
> which doesn't support this parameter itself." with another one, like:
> "If the driver supports this feature by itself, be carefully not to enable
> the watchdog twice".
> 
> What do you think?
> 

I am still missing the explanation _why_ it would be important not to enable
a watchdog twice. Why does it matter ? What is the difference ?

If there is a concern that the start function should not be called on an already
running watchdog, the code could check for that and ensure that WDOG_HW_RUNNING
is not already set before enabling it. That would probably make sense anyway.
But adding a limitation/restriction like the above, which is not enforceable,
is not a good idea. How would the common user know if a watchdog is already
running (eg because it was started in BIOS/ROMMON) ?

Thanks,
Guenter

Re: Linux 5.12-rc7

[Guenter Roeck]

On Sun, Apr 11, 2021 at 03:41:18PM -0700, Linus Torvalds wrote:
> Oh well. rc5 was big. rc6 was small. And now rc7 is big again. In
> fact, it's the biggest rc7 (at least in number of commits) we've had
> in the 5.x series.
> 
> It's mostly due to networking fixes (of which rc6 had none), and none
> of them should be all that scary, but it's never great when we have
> such a big rc. It's particularly annoying at the end of the release
> window like this.
> 
> End result: I'm still waffling about the final 5.12 release.  The fact
> that we have a big rc7 does make me think that I'll probably do an rc8
> this time around. But it ends up depending a bit on how the upcoming
> week goes, and if things are deathly quiet, I may end up deciding that
> an rc8 doesn't really make sense.
> 
> So we'll see.
> 
> Anyway, networking (both core and drivers) is over half of the rc7
> patch, with the rest being a fairly random collection of fixes all
> over. We've got other driver updates (sound, rdma, scsi, usb..) some
> fs fixes (io_uring, umount, btrfs, cifs, ocfs), minor arch fixes (arc,
> arm, parisc, powerpc, s390, x86), and other misc fixes.
> 
> The shortlog is appended, although it's obviously not as nice and
> small and readable as I'd have liked at this point in the release..
> 
> Please do test,
> 

Build results:
total: 151 pass: 151 fail: 0
Qemu test results:
total: 460 pass: 459 fail: 1
Failed tests:
sh:rts7751r2dplus_defconfig:ata:net,virtio-net:rootfs

The failure bisects to commit 0f6925b3e8da ("virtio_net: Do not pull payload in
skb->head"). It is a spurious problem - the test passes roughly every other
time. When the failure is seen, udhcpc fails to get an IP address and aborts
with SIGTERM. So far I have only seen this with the "sh" architecture.

Guenter

Re: [PATCH] ASoC: fsl_sai: Don't use devm_regmap_init_mmio_clk

[Guenter Roeck]

On Fri, Mar 19, 2021 at 04:06:43PM +0800, Shengjiu Wang wrote:
> When there is power domain bind with bus clock,
> 
> The call flow:
> devm_regmap_init_mmio_clk
>- clk_prepare()
>   - clk_pm_runtime_get()
> 
> cause the power domain of clock always be enabled after
> regmap_init(). which impact the power consumption.
> 
> So use devm_regmap_init_mmio instead of
> devm_regmap_init_mmio_clk, then explicitly enable clock when
> using by pm_runtime_get(), if CONFIG_PM=n, then
> fsl_sai_runtime_resume will be explicitly called.
> 
> Signed-off-by: Shengjiu Wang 
> Signed-off-by: Viorel Suman 

This patch results in a crash when running mcimx6ul-evk in qemu.
Reverting it fixes the problem.

Crash and bisect logs attached.

Guenter

---
[   19.196778] 8<--- cut here ---
[   19.197011] Unhandled fault: external abort on non-linefetch (0x808) at 
0xd1588000
[   19.197135] pgd = (ptrval)
[   19.197203] [d1588000] *pgd=824da811, *pte=0202c653, *ppte=0202c453
[   19.197764] Internal error: : 808 [#1] SMP ARM
[   19.197953] Modules linked in:
[   19.198108] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.12.0-rc6-next-20210409 #1
[   19.198234] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[   19.198354] PC is at regmap_mmio_write32le+0x24/0x2c
[   19.198482] LR is at regmap_mmio_write32le+0x1c/0x2c
[   19.198544] pc : []lr : []psr: 6093
[   19.198611] sp : c20b5cf0  ip :   fp : c017a344
[   19.198669] r10: c217c1b0  r9 : c20b4000  r8 : c26fcc00
[   19.198729] r7 : 0100  r6 : c26ff580  r5 :   r4 : 0100
[   19.198801] r3 : d1588000  r2 : 0100  r1 : d1588000  r0 : c26ff580
[   19.198896] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment 
none
[   19.198982] Control: 10c5387d  Table: 826bc06a  DAC: 0051
[   19.199060] Register r0 information: slab kmalloc-64 start c26ff580 pointer 
offset 0 size 64
[   19.199421] Register r1 information: 0-page vmalloc region starting at 
0xd1588000 allocated at __devm_ioremap+0x90/0xa4
[   19.199587] Register r2 information: non-paged memory
[   19.199667] Register r3 information: 0-page vmalloc region starting at 
0xd1588000 allocated at __devm_ioremap+0x90/0xa4
[   19.199774] Register r4 information: non-paged memory
[   19.199832] Register r5 information: NULL pointer
[   19.199888] Register r6 information: slab kmalloc-64 start c26ff580 pointer 
offset 0 size 64
[   19.18] Register r7 information: non-paged memory
[   19.200056] Register r8 information: slab kmalloc-1k start c26fcc00 pointer 
offset 0 size 1024
[   19.200167] Register r9 information: non-slab/vmalloc memory
[   19.200252] Register r10 information: slab kmalloc-1k start c217c000 pointer 
offset 432 size 1024
[   19.200367] Register r11 information: non-slab/vmalloc memory
[   19.200431] Register r12 information: NULL pointer
[   19.200495] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[   19.200596] Stack: (0xc20b5cf0 to 0xc20b6000)
[   19.200755] 5ce0: c26ff580  
0100 c0965f40
[   19.200932] 5d00: c26fcc00   c095f3cc c20c c26fcc00 
 0100
[   19.201096] 5d20: c269b840  c20b4000 c217c1b0 c017a344 c0961130 
0080 c217c010
[   19.201259] 5d40: c26ff5c0 c0d21354 c217c010 c0946e24  c0946e24 
c217c114 c094a894
[   19.201420] 5d60: c217c010 c0946e24 c2173810 c217c114 c2173914 c20b4000 
c217c1b0 c094a930
[   19.201582] 5d80: c217c010 c0946e24 c2173810 c094a468 c20c c20b4000 
c094a6a0 6013
[   19.201744] 5da0: 0002 cbdc8024 c217c114 5bdc6b72 6013 6013 
c217c114 0004
[   19.201906] 5dc0: 0002 cbdc8024 c20b4000 c269b880  c094a6b4 
c269b840 c217c010
[   19.202067] 5de0: c217c000 c0d2176c     
c2176340 c21d5c00
[   19.202228] 5e00:  6b6c636d 0033 5bdc6b72   
c217c010 c18d47fc
[   19.202389] 5e20: c1f70c20  c18d47fc   c093f540 
c217c010 c1f70c1c
[   19.202551] 5e40:  c1f70c20  c093cdec c217c010 c18d47fc 
c18d47fc c20b4000
[   19.202712] 5e60:  c166e854 c20af880 c093d0fc c217c010  
c18d47fc c093d418
[   19.202873] 5e80:  c18d47fc c217c010 c093d484  c18d47fc 
c093d420 c093aefc
[   19.203035] 5ea0: c26fe980 c20ae2b0 c214be94 5bdc6b72 c20ae2e4 c18d47fc 
c26fe980 
[   19.203196] 5ec0: c187bcf8 c093c0b8 c14db3fc c1661678 c18f4c20 c18d47fc 
c1661678 c18f4c20
[   19.203357] 5ee0: c17093d0 c093e1e4 c20b4000 c1661678 c18f4c20 c01022b0 
 
[   19.203533] 5f00: c20af8ec  c17e0b10 c0f39294 c17093d0  
c14a1cb8 c20b4000
[   19.203696] 5f20: c18f4c20 c17093d0 c15644e0 c18ff000 c166e854 01c6 
 c01af994
[   19.203858] 5f40:  5bdc6b72 c168dca8 0007 c166e874 c15644e0 
c18ff000 c166e854
[   19.204019] 5f60: c20af880 c16010a0 0006 0006  c16003e8 
c0f46080 01c6
[   19.204180] 5f80: c17097d0  c0f3a784   

Re: [PATCH v4 2/2] usb: dwc3: Add driver for Xilinx platforms

[Guenter Roeck]

Michal,

On 4/7/21 11:08 PM, Michal Simek wrote:
...
> It looks like that you directly created the patch. Isn't it better to
> send it yourself? Or do you want Manish to create it based on guidance
> above?
> 
-next is substantially broken all over the place. I already spend way too much
time bisecting and analyzing the failures, and making sure that the problems
are not caused by qemu (which is why I tracked down this problem in such 
detail).
I don't really have time to write patches and guide them through the process,
sorry.

Guenter

Re: [PATCH v2] hwmon: (dell-smm) Add Dell Latitude E7440 to fan control whitelist

[Guenter Roeck]

On Sun, Apr 11, 2021 at 11:57:41AM +0200, Sebastian Oechsle wrote:
> Allow manual PWM control on Dell Latitude E7440.
> 
> Signed-off-by: Sebastian Oechsle 
> 
> Changes in v2:
> - Fix spelling
> - Fix format
> ---

Applied, but next time changelog goes here, please (after ---).

Thanks,
Guenter

>  drivers/hwmon/dell-smm-hwmon.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 73b9db9e3aab..2970892bed82 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1210,6 +1210,14 @@ static struct dmi_system_id 
> i8k_whitelist_fan_control[] __initdata = {
>   },
>   .driver_data = (void *)_fan_control_data[I8K_FAN_34A3_35A3],
>   },
> + {
> + .ident = "Dell Latitude E7440",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Latitude E7440"),
> + },
> + .driver_data = (void *)_fan_control_data[I8K_FAN_34A3_35A3],
> + },
>   { }
>  };
>  

Re: [PATCH v4] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On 4/10/21 11:18 AM, Thomas Weißschuh wrote:
> Changes since v3:
> * Completely hide unusable sensors
> * Expose force_load parameter read-only via sysfs
> * Naming
> * Style cleanups
> 
> -- >8 --
> 
> Tested with
> * X570 I Aorus Pro Wifi (rev 1.0)
> * B550M DS3H
> * B550 Gaming X V2 (rev.1.x)
> * Z390 I AORUS PRO WIFI (rev. 1.0)
> 
> The mainboard contains an ITE IT8688E chip for management.
> This chips is also handled by drivers/hwmon/i87.c but as it is also used
> by the firmware itself it needs an ACPI driver.
> 
> Unfortunately not all sensor registers are handled by the firmware and even
> less are exposed via WMI.
> 
> Signed-off-by: Thomas Weißschuh 

Good enough, though you may want to improve your description.

Reviewed-by: Guenter Roeck 

FWIW, on B450 AORUS M:

gigabyte-wmi DEADBEEF-2001--00A0-C9062910: Forcing load on unknown 
platform
gigabyte-wmi DEADBEEF-2001--00A0-C9062910: No temperature sensors usable

Wonder who came up with that GUID.

> ---

Change log and everything that is not supposed to show up in the git history
is supposed to go here.

Guenter

>  MAINTAINERS |   6 +
>  drivers/platform/x86/Kconfig|  11 ++
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/gigabyte-wmi.c | 195 
>  4 files changed, 213 insertions(+)
>  create mode 100644 drivers/platform/x86/gigabyte-wmi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d92f85ca831d..9c10cfc00fe8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2*
>  F:   fs/gfs2/
>  F:   include/uapi/linux/gfs2_ondisk.h
>  
> +GIGABYTE WMI DRIVER
> +M:   Thomas Weißschuh 
> +L:   platform-driver-...@vger.kernel.org
> +S:   Maintained
> +F:   drivers/platform/x86/gigabyte-wmi.c
> +
>  GNSS SUBSYSTEM
>  M:   Johan Hovold 
>  S:   Maintained
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index ad4e630e73e2..96622a2106f7 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -123,6 +123,17 @@ config XIAOMI_WMI
> To compile this driver as a module, choose M here: the module will
> be called xiaomi-wmi.
>  
> +config GIGABYTE_WMI
> + tristate "Gigabyte WMI temperature driver"
> + depends on ACPI_WMI
> + depends on HWMON
> + help
> +   Say Y here if you want to support WMI-based temperature reporting on
> +   Gigabyte mainboards.
> +
> +   To compile this driver as a module, choose M here: the module will
> +   be called gigabyte-wmi.
> +
>  config ACERHDF
>   tristate "Acer Aspire One temperature and fan driver"
>   depends on ACPI && THERMAL
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 60d554073749..1621ebfd04fd 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += 
> intel-wmi-thunderbolt.o
>  obj-$(CONFIG_MXM_WMI)+= mxm-wmi.o
>  obj-$(CONFIG_PEAQ_WMI)   += peaq-wmi.o
>  obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> +obj-$(CONFIG_GIGABYTE_WMI)   += gigabyte-wmi.o
>  
>  # Acer
>  obj-$(CONFIG_ACERHDF)+= acerhdf.o
> diff --git a/drivers/platform/x86/gigabyte-wmi.c 
> b/drivers/platform/x86/gigabyte-wmi.c
> new file mode 100644
> index ..c17e51fcf000
> --- /dev/null
> +++ b/drivers/platform/x86/gigabyte-wmi.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  Copyright (C) 2021 Thomas Weißschuh 
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define GIGABYTE_WMI_GUID"DEADBEEF-2001--00A0-C9062910"
> +#define NUM_TEMPERATURE_SENSORS  6
> +
> +static bool force_load;
> +module_param(force_load, bool, 0444);
> +MODULE_PARM_DESC(force_load, "Force loading on unknown platform");
> +
> +static u8 usable_sensors_mask;
> +
> +enum gigabyte_wmi_commandtype {
> + GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
> + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
> + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
> + GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
> + GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
> +};
> +
> +struct gigabyte_wmi_args {
> + u32 arg1;
> +};
> +
> +static int gigabyte_wmi_perform_query(struct wmi_device *wdev,
> +   enum gigabyte_wmi_com

Re: [PATCH 08/10] powerpc/signal32: Convert restore_[tm]_user_regs() to user access block

[Guenter Roeck]

On Fri, Mar 19, 2021 at 11:06:57AM +, Christophe Leroy wrote:
> Convert restore_user_regs() and restore_tm_user_regs()
> to use user_access_read_begin/end blocks.
> 
> Signed-off-by: Christophe Leroy 
> ---
...
>  static long restore_user_regs(struct pt_regs *regs,
> struct mcontext __user *sr, int sig)
>  {
...
> @@ -567,19 +569,22 @@ static long restore_user_regs(struct pt_regs *regs,
>   regs->msr &= ~MSR_SPE;
>   if (msr & MSR_SPE) {
>   /* restore spe registers from the stack */
> - if (__copy_from_user(current->thread.evr, >mc_vregs,
> -  ELF_NEVRREG * sizeof(u32)))
> - return 1;
> + unsafe_copy_from_user(current->thread.evr, >mc_vregs,
> +   ELF_NEVRREG * sizeof(u32));

arch/powerpc/kernel/signal_32.c: In function 'restore_user_regs':
arch/powerpc/kernel/signal_32.c:565:36: error: macro "unsafe_copy_from_user" 
requires 4 arguments, but only 3 given

Guenter

Re: [PATCH 4.14 00/14] 4.14.230-rc1 review

[Guenter Roeck]

On 4/10/21 2:21 AM, Naresh Kamboju wrote:
> On Sat, 10 Apr 2021 at 01:43, Guenter Roeck  wrote:
>>
>> On Fri, Apr 09, 2021 at 11:53:25AM +0200, Greg Kroah-Hartman wrote:
>>> This is the start of the stable review cycle for the 4.14.230 release.
>>> There are 14 patches in this series, all will be posted as a response
>>> to this one.  If anyone has any issues with these being applied, please
>>> let me know.
>>>
>>> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
>>> Anything received after that time might be too late.
>>>
>>
>> Build results:
>> total: 168 pass: 168 fail: 0
>> Qemu test results:
>> total: 408 pass: 408 fail: 0
>>
>> Tested-by: Guenter Roeck 
>>
>> Having said this, I did see a spurious crash, and I see an unusual warning.
>> I have seen the crash only once, but the warning happens with every boot.
>> These are likely not new but exposed because I added network interface
>> tests. This is all v4.14.y specific; I did not see it in other branches.
>> See below for the tracebacks. Maybe someone has seen it before.
> 
> I do not notice these warnings.
> Please share the testing environment / device / setup / network interfaces
> and Kernel configs and steps to reproduce.
> 
>  - Naresh
> 

Hi Naresh,

the configuration is based on aspeed_g5_defconfig (see attached configuration 
file)
and the following qemu command line (qemu v5.2):

qemu-system-arm -M romulus-bmc -kernel \
arch/arm/boot/zImage -no-reboot \
-initrd rootfs-armv5.cpio -nic user -nodefaults \
--append "panic=-1 slub_debug=FZPUA rdinit=/sbin/init 
console=ttyS4,115200 earlycon=uart8250,mmio32,0x1e784000,115200n8" \
-dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
-nographic -monitor null -serial stdio

It also happens with other bmc platforms using the same network interface, 
though
it is for some reason more prevalent on romulus-bmc.

The root file system is generated with buildroot. See buildone.sh / buildall.sh
in branch local-2021.02 of g...@github.com:groeck/buildroot.git. The warning 
happens
with almost every boot; the crash in maybe one of 5-10 boots. The test, if you 
want
to call it that, uses udhcpc to get an IP address and then

net_test_successful=0
ifconfig eth0 2>/dev/null | grep -q "inet addr:10.0.2.15"
if [ $? -eq 0 ]; then
ping -q -c 1 -s 1000 -W 1 -I eth0 10.0.2.2 >/dev/null
if [ $? -eq 0 ]; then
telnet 10.0.2.2:22 /dev/null 2>dev/null
if [ $? -eq 0 ]; then
net_test_successful=1
fi
fi
fi

See package/busybox/run.sh in the above repository/branch.

Guenter


defconfig.gz
Description: application/gzip

Re: [PATCH v3] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On 4/10/21 7:57 AM, Hans de Goede wrote:
> Hi,
> 
> On 4/10/21 4:40 PM, Thomas Weißschuh wrote:
>> Changes since v1:
>> * Incorporate feedback from Barnabás Pőcze
>>   * Use a WMI driver instead of a platform driver
>>   * Let the kernel manage the driver lifecycle
>>   * Fix errno/ACPI error confusion
>>   * Fix resource cleanup
>>   * Document reason for integer casting
>>
>> Changes since v2:
>> * Style cleanups
>> * Test for usability during probing
>> * DMI-based whitelist
>> * CC hwmon maintainers
>>
>> -- >8 --
>>
>> Tested with a X570 I Aorus Pro Wifi.
>> The mainboard contains an ITE IT8688E chip for management.
>> This chips is also handled by drivers/hwmon/i87.c but as it is also used
>> by the firmware itself it needs an ACPI driver.
>>
>> Unfortunately not all sensor registers are handled by the firmware and even
>> less are exposed via WMI.
>>
>> Signed-off-by: Thomas Weißschuh 
> 
> This looks good, one small nitpick:
> 
> I know this is a touchy subject for some, but we are trying to move away
> from the whitelist/blacklist naming where possible and we don't want to
> introduce any new cases, see:
> 
> https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst#4-naming
> 
> The driver currently uses this twice:
> "Force loading on non-whitelisted platform"
> "Forcing loading on non-whitelisted platform"
> 
> Interestingly enough you already avoided naming the dmi_system_id table
> a whitelist (good).
> 
> I would like to see "non-whitelisted" replaced with "unknown" so that we end 
> up with:
> 
> "Force loading on unknown platform"
> "Forcing loading on unknown platform"
> 
> And while at it, I think for the second sentence this would be better English
> (I'm not a native speaker myself):
> 
> "Forcing load on unknown platform"
> 

Not native either, but I think it is either "Forcing load" or "Force loading".

> If you are ok with these changes I can fix this up while merging, no need
> to send a v4. Although if you prefer to send a v4 that is fine too.
> 

Please consider adding an existence check into the is_visible function.
Sysfs attributes for non-existing sensors should not be created.

Thanks,
Guenter

> Either way let me know.
> 
> Regards,
> 
> Hans
> 
> p.s.
> 
> For v4 or for a next patch, the way to add the changelog so that it does
> not get picked up / automatically gets snipped by git am is to put it
> below the Signed-off-by at the end of the commit message like this:
> 
> Signed-off-by: Thomas Weißschuh 
> ---
> Changes since v1:
> * Incorporate feedback from Barnabás Pőcze
>   * Use a WMI driver instead of a platform driver
>   * Let the kernel manage the driver lifecycle
>   * Fix errno/ACPI error confusion
>   * Fix resource cleanup
>   * Document reason for integer casting
>  
> Changes since v2:
> * Style cleanups
> * Test for usability during probing
> * DMI-based whitelist
> * CC hwmon maintainers
> 
> 
> 
> 
> 
> 
> 
>> ---
>>  MAINTAINERS |   6 +
>>  drivers/platform/x86/Kconfig|  11 ++
>>  drivers/platform/x86/Makefile   |   1 +
>>  drivers/platform/x86/gigabyte-wmi.c | 194 
>>  4 files changed, 212 insertions(+)
>>  create mode 100644 drivers/platform/x86/gigabyte-wmi.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d92f85ca831d..9c10cfc00fe8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7543,6 +7543,12 @@ F:Documentation/filesystems/gfs2*
>>  F:  fs/gfs2/
>>  F:  include/uapi/linux/gfs2_ondisk.h
>>  
>> +GIGABYTE WMI DRIVER
>> +M:  Thomas Weißschuh 
>> +L:  platform-driver-...@vger.kernel.org
>> +S:  Maintained
>> +F:  drivers/platform/x86/gigabyte-wmi.c
>> +
>>  GNSS SUBSYSTEM
>>  M:  Johan Hovold 
>>  S:  Maintained
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index ad4e630e73e2..96622a2106f7 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -123,6 +123,17 @@ config XIAOMI_WMI
>>To compile this driver as a module, choose M here: the module will
>>be called xiaomi-wmi.
>>  
>> +config GIGABYTE_WMI
>> +tristate "Gigabyte WMI temperature driver"
>> +depends on ACPI_WMI
>> +depends on HWMON
>> +help
>> +  Say Y here if you want to support WMI-based temperature reporting on
>> +  Gigabyte mainboards.
>> +
>> +  To compile this driver as a module, choose M here: the module will
>> +  be called gigabyte-wmi.
>> +
>>  config ACERHDF
>>  tristate "Acer Aspire One temperature and fan driver"
>>  depends on ACPI && THERMAL
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index 60d554073749..1621ebfd04fd 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT)+= 
>> intel-wmi-thunderbolt.o
>>  obj-$(CONFIG_MXM_WMI)   += mxm-wmi.o
>>  obj-$(CONFIG_PEAQ_WMI)  += peaq-wmi.o

Re: [PATCH v3] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On 4/10/21 7:40 AM, Thomas Weißschuh wrote:
> Changes since v1:
> * Incorporate feedback from Barnabás Pőcze
>   * Use a WMI driver instead of a platform driver
>   * Let the kernel manage the driver lifecycle
>   * Fix errno/ACPI error confusion
>   * Fix resource cleanup
>   * Document reason for integer casting
> 
> Changes since v2:
> * Style cleanups
> * Test for usability during probing
> * DMI-based whitelist
> * CC hwmon maintainers
> 
> -- >8 --
> 
> Tested with a X570 I Aorus Pro Wifi.
> The mainboard contains an ITE IT8688E chip for management.
> This chips is also handled by drivers/hwmon/i87.c but as it is also used
> by the firmware itself it needs an ACPI driver.
> 
> Unfortunately not all sensor registers are handled by the firmware and even
> less are exposed via WMI.
> 
> Signed-off-by: Thomas Weißschuh 
> ---
>  MAINTAINERS |   6 +
>  drivers/platform/x86/Kconfig|  11 ++
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/gigabyte-wmi.c | 194 
>  4 files changed, 212 insertions(+)
>  create mode 100644 drivers/platform/x86/gigabyte-wmi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d92f85ca831d..9c10cfc00fe8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2*
>  F:   fs/gfs2/
>  F:   include/uapi/linux/gfs2_ondisk.h
>  
> +GIGABYTE WMI DRIVER
> +M:   Thomas Weißschuh 
> +L:   platform-driver-...@vger.kernel.org
> +S:   Maintained
> +F:   drivers/platform/x86/gigabyte-wmi.c
> +
>  GNSS SUBSYSTEM
>  M:   Johan Hovold 
>  S:   Maintained
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index ad4e630e73e2..96622a2106f7 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -123,6 +123,17 @@ config XIAOMI_WMI
> To compile this driver as a module, choose M here: the module will
> be called xiaomi-wmi.
>  
> +config GIGABYTE_WMI
> + tristate "Gigabyte WMI temperature driver"
> + depends on ACPI_WMI
> + depends on HWMON
> + help
> +   Say Y here if you want to support WMI-based temperature reporting on
> +   Gigabyte mainboards.
> +
> +   To compile this driver as a module, choose M here: the module will
> +   be called gigabyte-wmi.
> +
>  config ACERHDF
>   tristate "Acer Aspire One temperature and fan driver"
>   depends on ACPI && THERMAL
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 60d554073749..1621ebfd04fd 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += 
> intel-wmi-thunderbolt.o
>  obj-$(CONFIG_MXM_WMI)+= mxm-wmi.o
>  obj-$(CONFIG_PEAQ_WMI)   += peaq-wmi.o
>  obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> +obj-$(CONFIG_GIGABYTE_WMI)   += gigabyte-wmi.o
>  
>  # Acer
>  obj-$(CONFIG_ACERHDF)+= acerhdf.o
> diff --git a/drivers/platform/x86/gigabyte-wmi.c 
> b/drivers/platform/x86/gigabyte-wmi.c
> new file mode 100644
> index ..fb4e6d4c1823
> --- /dev/null
> +++ b/drivers/platform/x86/gigabyte-wmi.c
> @@ -0,0 +1,194 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  Copyright (C) 2021 Thomas Weißschuh 
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910"
> +#define NUM_TEMPERATURE_SENSORS 6

Style: #definenamevalue

but of course that is Hans' call.

> +
> +static bool force_load;
> +module_param(force_load, bool, 0);
> +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform");
> +
> +enum gigabyte_wmi_commandtype {
> + GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
> + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
> + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
> + GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
> + GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
> +};
> +
> +struct gigabyte_wmi_args {
> + u32 arg1;
> +};
> +
> +static int gigabyte_wmi_perform_query(struct wmi_device *wdev,
> +   enum gigabyte_wmi_commandtype command,
> +   struct gigabyte_wmi_args *args, struct 
> acpi_buffer *out)
> +{
> + const struct acpi_buffer in = {
> + .length = sizeof(*args),
> + .pointer = args,
> + };
> +
> + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, , out);
> +
> + if ACPI_FAILURE(ret)
> + return -EIO;
> +
> + return 0;
> +}
> +
> +static int gigabyte_wmi_query_integer(struct wmi_device *wdev,
> +   enum gigabyte_wmi_commandtype command,
> +   struct gigabyte_wmi_args *args, u64 *res)
> +{
> + union acpi_object *obj;
> + struct 

Re: [PATCH 4.14 00/14] 4.14.230-rc1 review

[Guenter Roeck]

Hi Greg,

On 4/10/21 6:15 AM, Greg Kroah-Hartman wrote:
> On Fri, Apr 09, 2021 at 01:13:06PM -0700, Guenter Roeck wrote:
>> On Fri, Apr 09, 2021 at 11:53:25AM +0200, Greg Kroah-Hartman wrote:
>>> This is the start of the stable review cycle for the 4.14.230 release.
>>> There are 14 patches in this series, all will be posted as a response
>>> to this one.  If anyone has any issues with these being applied, please
>>> let me know.
>>>
>>> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
>>> Anything received after that time might be too late.
>>>
>>
>> Build results:
>>  total: 168 pass: 168 fail: 0
>> Qemu test results:
>>  total: 408 pass: 408 fail: 0
>>
>> Tested-by: Guenter Roeck 
>>
>> Having said this, I did see a spurious crash, and I see an unusual warning.
>> I have seen the crash only once, but the warning happens with every boot.
>> These are likely not new but exposed because I added network interface
>> tests. This is all v4.14.y specific; I did not see it in other branches.
>> See below for the tracebacks. Maybe someone has seen it before.
> 
> Thanks for testing all of these, I'll go queue up your reported fixes
> here for the next release.
> 

You'll need one more patch to avoid yet another warning:

b0949618826c net/ncsi: Avoid GFP_KERNEL in response handler

Thanks,
Guenter

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On 4/8/21 11:02 PM, Thomas Weißschuh wrote:
> On Do, 2021-04-08T08:00-0700, Guenter Roeck wrote:
>> On 4/8/21 2:36 AM, Hans de Goede wrote:
>>> On 4/7/21 9:43 PM, Thomas Weißschuh wrote:
>>>> On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote:
>>> Jean, Guenter,
>>>
>>> Thomas has been working on a WMI driver to expose various motherboard
>>> temperatures on a gigabyte board where the IO-addresses for the it87 chip
>>> are reserved by ACPI. We are discussing how best to deal with this, there
>>> are some ACPI methods to directly access the super-IO registers (with 
>>> locking
>>> to protect against other ACPI accesses). This reminded me of an idea I had
>>> a while ago to solve a similar issue with an other superIO chip, abstract
>>> the superIO register access-es using some reg_ops struct and allow an 
>>> ACPI/WMI
>>> driver to provide alternative reg_ops:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=204807#c47
>>>
>>> Do you think this is a good idea (or a bad one)? And would something like 
>>> that
>>> be acceptable to you ?
>>>
>>
>> The upstream it87 driver is severely out of date. I had an out-of-tree driver
>> with various improvements which I didn't upstream, first because no one was 
>> willing
>> to review changes and then because it had deviated too much. I pulled it from
>> public view because I got pounded for not upstreaming it, because people 
>> started
>> demanding support (not asking, demanding) for it, and because Gigabyte 
>> stopped
>> providing datasheets for the more recent ITE chips and it became effectively
>> unmaintainable.
>>
>> Some ITE chips have issues which can cause system hangs if accessed directly.
>> I put some work to remedy that into the non-upstream driver, but that was all
>> just guesswork. Gigabyte knows about the problem (or so I was told from 
>> someone
>> who has an NDA with them), but I didn't get them or ITE to even acknowledge 
>> it
>> to me. I even had a support case open with Gigabyte for a while, but all I 
>> could
>> get out of them is that they don't support Linux and what I would have to 
>> reproduce
>> the problem with Windows for them to provide assistance (even though, again,
>> they knew about it).
>>
>> As for using ACPI locks or WMI to ensure that ACPI leaves the chip alone 
>> while
>> the driver accesses chips directly: That is an option, but it has (at least)
>> two problems.
>>
>> First, ACPI access methods are not well documented or standardized. I had 
>> tried
>> to find useful means to do that some time ago, but I gave up because each 
>> board
>> (even from the same vendor) handles locking and accesses differently. We 
>> would
>> end up with lots of board specific code. Coincidentally, that was for ASUS 
>> boards
>> and the nct6775 driver.
> 
> At least for all the Gigabyte ACPI tables I have looked at all access is done
> via two-byte "OperationRegion" over the Index/Data addresses, a "Field" with
> two entries for these and an "IndexField" that is actually used to perform the
> accesses.
> As the IndexField is synchronized via "Lock" it should take a lock on the
> OperationRegion itself.
> 
> So I think we should be technically fine with validating these assumption and
> then also taking locks on the OperationRegion.
> 
> If it is reasonable to do so is another question.
> 
You'd still have to validate this for each individual board unless you get
confirmation from Gigabyte that the mechanism is consistent on their boards.
Then you'd have to handle other vendors using it87 chips, and those are
just as close-lipped as Gigabyte. Ultimately it would require acpi match
tables to match the various boards and access methods. I had experimented
with this this a long time ago but gave up on it after concluding that it was
unmaintainable.

>> Second, access through ACPI is only one of the issues. Turns out there are 
>> two
>> ITE chips on many of the Gigabyte boards nowadays, and the two chips talk to 
>> each
>> other using I2C. My out-of-tree driver tried to remedy that by blocking those
>> accesses while the driver used the chip, but, again, without Gigabyte / ITE
>> support this was never a perfect solution, and there was always the risk that
>> the board ended up hanging because that access was blocked for too long.
>> Recent ITE chips solve that problem by providing memory mapped access to the
>> chip registers, but that is only useful if one has a datasheet.
> 

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On 4/8/21 9:07 AM, Hans de Goede wrote:
> Hi Guenter,
> 
> On 4/8/21 5:08 PM, Guenter Roeck wrote:
>> On Mon, Apr 05, 2021 at 10:48:10PM +0200, Thomas Weißschuh wrote:
>>> Changes since v1:
>>> * Incorporate feedback from Barnabás Pőcze
>>>   * Use a WMI driver instead of a platform driver
>>>   * Let the kernel manage the driver lifecycle
>>>   * Fix errno/ACPI error confusion
>>>   * Fix resource cleanup
>>>   * Document reason for integer casting
>>>
>>> Thank you Barnabás for your review, it is much appreciated.
>>>
>>> -- >8 --
>>>
>>> Tested with a X570 I Aorus Pro Wifi.
>>> The mainboard contains an ITE IT8688E chip for management.
>>> This chips is also handled by drivers/hwmon/i87.c but as it is also used
>>> by the firmware itself it needs an ACPI driver.
>>>
>>> Unfortunately not all sensor registers are handled by the firmware and even
>>> less are exposed via WMI.
>>>
>>> Signed-off-by: Thomas Weißschuh 
>>> ---
>>>  drivers/platform/x86/Kconfig|  11 +++
>>>  drivers/platform/x86/Makefile   |   1 +
>>>  drivers/platform/x86/gigabyte-wmi.c | 138 
>>
>> Originally drivers/platform was supposed to be used for platform specific
>> code. Not that I have control over it, but I really dislike that more and
>> more hwmon drivers end up there.
>>
>> At least hwmon is in good company - I see drivers for various other 
>> subsystems
>> there as well. I just wonder if that is such a good idea. That entire 
>> directory
>> is bypassing subsystem maintainer reviews.
> 
> In case you are not aware I've recent(ish) taken over the drivers/platform/x86
> maintainership from Andy Shevchenko.
> 
> Yes it is a bit of an odd grab-bag it mostly deals with vendor specific
> ACPI / WMI interfaces which often more or less require using a single
> driver while offering multiple functionalities. These firmware interfaces
> do not really lend themselves to demultiplexing through something like
> MFD. These are mostly found on laptops where they deal with some or all of:
> 
> - Hotkeys for brightness adjust / wlan-on/off toggle, touchpad on/off toggle, 
> etc.
>   (input subsystem stuff)
> - Mic. / Speaker mute LEDS (and other special LEDs) found on some laptops
>   (LED subsystem stuff)
> - Enabling/disabling radios
>   (rfkill stuff)
> - Controlling the DPTF performance profile
>   (ACPI stuff)
> - Various sensors, some hwmon, some IIO
> - Backlight control (drm/kms subsys)
> - Enabling/disabling of LCD-builtin privacy filters (requires KMS/DRM subsys 
> integration, pending)
> - Fan control (hwmon subsys)
> 
> And often all of this in a single driver. This is all "stuff" for which
> there are no standard APIs shared between vendors, so it is a free for
> all and often it is all stuffed behind a single WMI or ACPI object,
> because that is how the vendor's drivers under Windows work.
> 
> It certainly is not my intention to bypass review by other subsystem
> maintainers and when there are significant questions I do try to always
> get other subsys maintainers involved. See e.g. this thread, but also the
> "[PATCH 1/3] thinkpad_acpi: add support for force_discharge" thread
> where I asked for input from sre for the power-supply aspects of that.
> 
> The WMI code was reworked a while back to make WMI be a bus and have
> individual WMI objects be devices on that bus. version 2 of this
> driver has been reworked to use this. Since this new driver is just a hwmon
> driver and as this is for a desktop I expect it will stay that way,
> I'm fine with moving this one over to drivers/hwmon if that has your
> preference.
> 
I thought about it, but I don't think it makes much sense since all other
WMI drivers are in drivers/platform.

> As for other cases then this driver, if you want to make sure you are at
> least Cc-ed on all hwmon related changes I'm fine with adding you as a
> reviewer to the pdx86 MAINTAINERS entry.
> 
I think I have a better idea: I'll add a regex pattern into the MAINTAINERS
entry for hardware monitoring drivers. This way we should get copied whenever
anyone adds a hardware monitoring driver into the tree.

Thanks,
Guenter

Re: [PATCH 4.14 00/14] 4.14.230-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 01:13:06PM -0700, Guenter Roeck wrote:
> On Fri, Apr 09, 2021 at 11:53:25AM +0200, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.14.230 release.
> > There are 14 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> > Anything received after that time might be too late.
> > 
> 
> Build results:
>   total: 168 pass: 168 fail: 0
> Qemu test results:
>   total: 408 pass: 408 fail: 0
> 
> Tested-by: Guenter Roeck 
> 
> Having said this, I did see a spurious crash, and I see an unusual warning.
> I have seen the crash only once, but the warning happens with every boot.
> These are likely not new but exposed because I added network interface
> tests. This is all v4.14.y specific; I did not see it in other branches.
> See below for the tracebacks. Maybe someone has seen it before.
> 
> Thanks,
> Guenter
> 
> ---
> ftgmac100 1e66.ethernet eth0: NCSI interface down
> [ cut here ]
> WARNING: CPU: 0 PID: 477 at drivers/base/dma-mapping.c:325 
> remap_allocator_free+0x54/0x5c
> trying to free invalid coherent area: 909a1000
> Modules linked in:
> CPU: 0 PID: 477 Comm: ip Not tainted 4.14.230-rc1-00015-gbbc0ac1df344 #1
> Hardware name: Generic DT based system
> [<8000f8dc>] (unwind_backtrace) from [<8000d194>] (show_stack+0x10/0x14)
> [<8000d194>] (show_stack) from [<805a5a80>] (__warn+0xc0/0xf4)
> [<805a5a80>] (__warn) from [<800177b4>] (warn_slowpath_fmt+0x38/0x48)
> [<800177b4>] (warn_slowpath_fmt) from [<80010554>] 
> (remap_allocator_free+0x54/0x5c)
> [<80010554>] (remap_allocator_free) from [<80010e4c>] 
> (__arm_dma_free.constprop.0+0xec/0x13c)
> [<80010e4c>] (__arm_dma_free.constprop.0) from [<80429924>] 
> (ftgmac100_free_rings+0x17c/0x1f8)
> [<80429924>] (ftgmac100_free_rings) from [<80429a24>] 
> (ftgmac100_stop+0x84/0xa4)
> [<80429a24>] (ftgmac100_stop) from [<804e8a70>] (__dev_close_many+0xac/0x100)
> [<804e8a70>] (__dev_close_many) from [<804f0dc0>] 
> (__dev_change_flags+0xb4/0x1a0)
> [<804f0dc0>] (__dev_change_flags) from [<804f0ec4>] 
> (dev_change_flags+0x18/0x48)
> [<804f0ec4>] (dev_change_flags) from [<80561644>] (devinet_ioctl+0x6cc/0x808)
> [<80561644>] (devinet_ioctl) from [<804d1548>] (sock_ioctl+0x188/0x2e4)
> [<804d1548>] (sock_ioctl) from [<800eac80>] (do_vfs_ioctl+0x3a0/0x82c)
> [<800eac80>] (do_vfs_ioctl) from [<800eb140>] (SyS_ioctl+0x34/0x60)
> [<800eb140>] (SyS_ioctl) from [<8000a600>] (ret_fast_syscall+0x0/0x28)
> ---[ end trace c13f2f82f69274ad ]---
> 
> =
> 
> ftgmac100 1e66.ethernet eth0: NCSI interface up
> Unable to handle kernel NULL pointer dereference at virtual address 
> pgd = 9ec84000
> [] *pgd=9f7f6831, *pte=, *ppte=
> Internal error: Oops: 17 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 397 Comm: default.script Not tainted 
> 4.14.230-rc1-00015-gbbc0ac1df344 #1
> Hardware name: Generic DT based system
> task: 9f5cc260 task.stack: 9ecee000
> PC is at anon_vma_clone+0x64/0x19c
> LR is at fs_reclaim_release+0x8/0x18
> pc : [<800c1ccc>]lr : [<80098b5c>]psr: a153
> sp : 9ecefe78  ip :   fp : 
> r10: 01000200  r9 : 9f7e6d10  r8 : 80cb9a44
> r7 : 9f7e0da0  r6 : 9f7e6d10  r5 : 9ed0f600  r4 : 9f5a562c
> r3 : 0030  r2 : 9fbdf618  r1 : 0034  r0 : 9ed0f600
> Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment user
> Control: 00c5387d  Table: 9ec84008  DAC: 0055
> Process default.script (pid: 397, stack limit = 0x9ecee188)
> Stack: (0x9ecefe78 to 0x9ecf)
> fe60:   9f5a303c 9f5a3000
> fe80: 0002 9f7e0da0 9f7e0ab4 9f5a3000 9f77e600 9f7e0da0 9f5a3000 9f77e400
> fea0: 9f72dc64 800c1e28 9f7e0ab0 9f7e0ab4 0002 9f77e600 9f7e0da0 800161f8
> fec0: 9f5cc640 cacd966c 9f5cc260 cd397f94 80cb0afc  80016870 
> fee0:  9f69f2f8 9f7e0aa8 807ca224 9f7e0aa0 9f72dc70 9f69f100 0011
> ff00: 9f77e658 9f77e458 9eceff08 9eceff08 9f5cc650 0011 7eb26888 
> ff20:   9ecee000  76eff3a0 80016870  
> ff40:  7eb26888 9eceff78  9ecee000 76ec4a28 7eb26888 9eceff78
> ff60:  7eb26888 0008  0008 800245e4 76efdcd0 7eb26888
> ff80:  0002 8000a704 9ecee000  80016cd4  
> 

Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available

[Guenter Roeck]

On 4/9/21 8:11 PM, 王擎 wrote:
> 
>> On 4/9/21 7:42 PM, 王擎 wrote:
>>>
 On 4/9/21 2:55 AM, Wang Qing wrote:
> Use the bark interrupt as the pretimeout notifier if available.
>
> By default, the pretimeout notification shall occur one second earlier
> than the timeout.
>
> Signed-off-by: Wang Qing 
> ---
>  drivers/watchdog/mtk_wdt.c | 47 
> +++---
>  1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 97ca993..8b919cc
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define WDT_MAX_TIMEOUT  31
>  #define WDT_MIN_TIMEOUT  1
> @@ -234,18 +235,35 @@ static int mtk_wdt_start(struct watchdog_device 
> *wdt_dev)
>   void __iomem *wdt_base = mtk_wdt->wdt_base;
>   int ret;
>  
> - ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> + ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - 
> wdt_dev->pretimeout);

 That looks suspiciously like the real watchdog won't happen at all.
 What will happen if the pretimeout governor is set to none ?

 Guenter

>>> The pretimeout governor is panic by default. If pretimeout is enabled and 
>>> the governor is
>>> set to none, it means the timeout behavior does not need to be processed, 
>>> only printing.
>>>
>>
>> That was not my question. My question was if the real timeout happens in 
>> that case.
>>
>> Guenter
>>
> Yes, the real timeout will happen. After WDT timeout, IRQ is sent out instead 
> of 
> reset signal first. In order to ensure that CPU does not get stuck after IRQ 
> is sent out, 
> WDT will time again and send reset signal to reset.
> 

When will that be, or in other words how does the chip know when to time out ?
After all, only a single timeout value is written into the chip. I don't see how
it would know to reset the chip after wdt_dev->timeout.

Guenter

Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available

[Guenter Roeck]

On 4/9/21 7:42 PM, 王擎 wrote:
> 
>> On 4/9/21 2:55 AM, Wang Qing wrote:
>>> Use the bark interrupt as the pretimeout notifier if available.
>>>
>>> By default, the pretimeout notification shall occur one second earlier
>>> than the timeout.
>>>
>>> Signed-off-by: Wang Qing 
>>> ---
>>>  drivers/watchdog/mtk_wdt.c | 47 
>>> +++---
>>>  1 file changed, 44 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
>>> index 97ca993..8b919cc
>>> --- a/drivers/watchdog/mtk_wdt.c
>>> +++ b/drivers/watchdog/mtk_wdt.c
>>> @@ -25,6 +25,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  
>>>  #define WDT_MAX_TIMEOUT31
>>>  #define WDT_MIN_TIMEOUT1
>>> @@ -234,18 +235,35 @@ static int mtk_wdt_start(struct watchdog_device 
>>> *wdt_dev)
>>> void __iomem *wdt_base = mtk_wdt->wdt_base;
>>> int ret;
>>>  
>>> -   ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
>>> +   ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - 
>>> wdt_dev->pretimeout);
>>
>> That looks suspiciously like the real watchdog won't happen at all.
>> What will happen if the pretimeout governor is set to none ?
>>
>> Guenter
>>
> The pretimeout governor is panic by default. If pretimeout is enabled and the 
> governor is
> set to none, it means the timeout behavior does not need to be processed, 
> only printing.
> 

That was not my question. My question was if the real timeout happens in that 
case.

Guenter

Re: [PATCH v2 4/6] usb: typec: tcpm: Honour pSnkStdby requirement during negotiation

[Guenter Roeck]

  PDO 0: type 0, 5000 mV, 3000 mA [E]
> [  169.077178]  PDO 1: type 0, 9000 mV, 2000 mA []
> [  169.077183] state change SNK_WAIT_CAPABILITIES -> 
> SNK_NEGOTIATE_CAPABILITIES [rev2 POWER_NEGOTIATION]
> [  169.077191] Setting usb_comm capable false
> [  169.077753] cc=0 cc1=0 cc2=5 vbus=0 vconn=sink polarity=1
> [  169.077759] Requesting PDO 1: 9000 mV, 2000 mA
> [  169.077762] PD TX, header: 0x1042
> [  169.079990] PD TX complete, status: 0
> [  169.080013] pending state change SNK_NEGOTIATE_CAPABILITIES -> 
> HARD_RESET_SEND @ 60 ms [rev2 POWER_NEGOTIATION]
> [  169.083183] VBUS on
> [  169.084195] PD RX, header: 0x363 [1]
> [  169.084200] state change SNK_NEGOTIATE_CAPABILITIES -> SNK_TRANSITION_SINK 
> [rev2 POWER_NEGOTIATION]
> [  169.084206] Setting standby current 5000 mV @ 500 mA
> [  169.084209] Setting voltage/current limit 5000 mV 500 mA
> [  169.084220] pending state change SNK_TRANSITION_SINK -> HARD_RESET_SEND @ 
> 500 ms [rev2 POWER_NEGOTIATION]
> [  169.260222] PD RX, header: 0x566 [1]
> [  169.260227] Setting voltage/current limit 9000 mV 2000 mA
> [  169.261315] set_auto_vbus_discharge_threshold mode:3 pps_active:n 
> vbus:9000 ret:0
> [  169.261321] state change SNK_TRANSITION_SINK -> SNK_READY [rev2 
> POWER_NEGOTIATION]
> [  169.261570] AMS POWER_NEGOTIATION finished
> 
> Fixes: f0690a25a140b ("staging: typec: USB Type-C Port Manager (tcpm)")
> Signed-off-by: Badhri Jagan Sridharan 

Reviewed-by: Guenter Roeck 

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 17 +
>  include/linux/usb/pd.h|  2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index d1d03ee90d8f..770b2edd9a04 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4131,6 +4131,23 @@ static void run_state_machine(struct tcpm_port *port)
>   }
>   break;
>   case SNK_TRANSITION_SINK:
> + /* From the USB PD spec:
> +  * "The Sink Shall transition to Sink Standby before a positive 
> or
> +  * negative voltage transition of VBUS. During Sink Standby
> +  * the Sink Shall reduce its power draw to pSnkStdby."
> +  *
> +  * This is not applicable to PPS though as the port can continue
> +  * to draw negotiated power without switching to standby.
> +  */
> + if (port->supply_voltage != port->req_supply_voltage && 
> !port->pps_data.active &&
> + port->current_limit * port->supply_voltage / 1000 > 
> PD_P_SNK_STDBY_MW) {
> + u32 stdby_ma = port->supply_voltage ? PD_P_SNK_STDBY_MW 
> * 1000 /
> + port->supply_voltage : 0;
> + tcpm_log(port, "Setting standby current %u mV @ %u mA",
> +  port->supply_voltage, stdby_ma);
> + tcpm_set_current_limit(port, stdby_ma, 
> port->supply_voltage);
> + }
> + fallthrough;
>   case SNK_TRANSITION_SINK_VBUS:
>   tcpm_set_state(port, hard_reset_state(port),
>  PD_T_PS_TRANSITION);
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index 70d681918d01..bf00259493e0 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -493,4 +493,6 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_N_CAPS_COUNT  (PD_T_NO_RESPONSE / 
> PD_T_SEND_SOURCE_CAP)
>  #define PD_N_HARD_RESET_COUNT2
>  
> +#define PD_P_SNK_STDBY_MW2500/* 2500 mW */
> +
>  #endif /* __LINUX_USB_PD_H */
> 

Re: [PATCH v2 3/6] usb: typec: tcpm: update power supply once partner accepts

[Guenter Roeck]

On 4/7/21 1:07 PM, Badhri Jagan Sridharan wrote:
> power_supply_changed needs to be called to notify clients
> after the partner accepts the requested values for the pps
> case.
> 
> Also, remove the redundant power_supply_changed at the end
> of the tcpm_reset_port as power_supply_changed is already
> called right after usb_type is changed.
> 
> Fixes: f2a8aa053c176 ("typec: tcpm: Represent source supply through 
> power_supply")
> Signed-off-by: Badhri Jagan Sridharan 
> Reviewed-by: Adam Thomson 

Reviewed-by: Guenter Roeck 

> ---
> Changes since V1:
> * Updated commit description to clarify Guenter Roeck's concern.
> * Added Reviewed-by tags
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index b4a40099d7e9..d1d03ee90d8f 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2568,6 +2568,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>   port->pps_data.max_curr = port->pps_data.req_max_curr;
>   port->req_supply_voltage = port->pps_data.req_out_volt;
>   port->req_current_limit = port->pps_data.req_op_curr;
> + power_supply_changed(port->psy);
>   tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
>   break;
>   case SOFT_RESET_SEND:
> @@ -3136,7 +3137,6 @@ static unsigned int tcpm_pd_select_pps_apdo(struct 
> tcpm_port *port)
> 
> port->pps_data.req_out_volt));
>   port->pps_data.req_op_curr = min(port->pps_data.max_curr,
>port->pps_data.req_op_curr);
> - power_supply_changed(port->psy);
>   }
>  
>   return src_pdo;
> @@ -3561,8 +3561,6 @@ static void tcpm_reset_port(struct tcpm_port *port)
>   port->sink_cap_done = false;
>   if (port->tcpc->enable_frs)
>   port->tcpc->enable_frs(port->tcpc, false);
> -
> - power_supply_changed(port->psy);
>  }
>  
>  static void tcpm_detach(struct tcpm_port *port)
> 

Re: [PATCH v2 2/6] usb: typec: tcpm: Address incorrect values of tcpm psy for pps supply

[Guenter Roeck]

On 4/7/21 1:07 PM, Badhri Jagan Sridharan wrote:
> tcpm_pd_select_pps_apdo overwrites port->pps_data.min_volt,
> port->pps_data.max_volt, port->pps_data.max_curr even before
> port partner accepts the requests. This leaves incorrect values
> in current_limit and supply_voltage that get exported by
> "tcpm-source-psy-". Solving this problem by caching the request
> values in req_min_volt, req_max_volt, req_max_curr, req_out_volt,
> req_op_curr. min_volt, max_volt, max_curr gets updated once the
> partner accepts the request. current_limit, supply_voltage gets updated
> once local port's tcpm enters SNK_TRANSITION_SINK when the accepted
> current_limit and supply_voltage is enforced.
> 
> Fixes: f2a8aa053c176 ("typec: tcpm: Represent source supply through 
> power_supply")
> Signed-off-by: Badhri Jagan Sridharan 
> Reviewed-by: Adam Thomson 

Reviewed-by: Guenter Roeck 

> ---
> Changes since V1:
> * Moved to kerneldoc header as suggested by Greg KH.
> * Added reviewed by tags.
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 88 +--
>  1 file changed, 53 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 4ea4b30ae885..b4a40099d7e9 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -268,12 +268,27 @@ struct pd_mode_data {
>   struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
>  };
>  
> +/*
> + * @min_volt: Actual min voltage at the local port
> + * @req_min_volt: Requested min voltage to the port partner
> + * @max_volt: Actual max voltage at the local port
> + * @req_max_volt: Requested max voltage to the port partner
> + * @max_curr: Actual max current at the local port
> + * @req_max_curr: Requested max current of the port partner
> + * @req_out_volt: Requested output voltage to the port partner
> + * @req_op_curr: Requested operating current to the port partner
> + * @supported: Parter has atleast one APDO hence supports PPS
> + * @active: PPS mode is active
> + */
>  struct pd_pps_data {
>   u32 min_volt;
> + u32 req_min_volt;
>   u32 max_volt;
> + u32 req_max_volt;
>   u32 max_curr;
> - u32 out_volt;
> - u32 op_curr;
> + u32 req_max_curr;
> + u32 req_out_volt;
> + u32 req_op_curr;
>   bool supported;
>   bool active;
>  };
> @@ -2498,8 +2513,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>   break;
>   case SNK_NEGOTIATE_PPS_CAPABILITIES:
>   /* Revert data back from any requested PPS updates */
> - port->pps_data.out_volt = port->supply_voltage;
> - port->pps_data.op_curr = port->current_limit;
> + port->pps_data.req_out_volt = port->supply_voltage;
> + port->pps_data.req_op_curr = port->current_limit;
>   port->pps_status = (type == PD_CTRL_WAIT ?
>   -EAGAIN : -EOPNOTSUPP);
>  
> @@ -2548,8 +2563,11 @@ static void tcpm_pd_ctrl_request(struct tcpm_port 
> *port,
>   break;
>   case SNK_NEGOTIATE_PPS_CAPABILITIES:
>   port->pps_data.active = true;
> - port->req_supply_voltage = port->pps_data.out_volt;
> - port->req_current_limit = port->pps_data.op_curr;
> + port->pps_data.min_volt = port->pps_data.req_min_volt;
> + port->pps_data.max_volt = port->pps_data.req_max_volt;
> + port->pps_data.max_curr = port->pps_data.req_max_curr;
> + port->req_supply_voltage = port->pps_data.req_out_volt;
> + port->req_current_limit = port->pps_data.req_op_curr;
>   tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
>   break;
>   case SOFT_RESET_SEND:
> @@ -3108,16 +3126,16 @@ static unsigned int tcpm_pd_select_pps_apdo(struct 
> tcpm_port *port)
>   src = port->source_caps[src_pdo];
>   snk = port->snk_pdo[snk_pdo];
>  
> - port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
> -   pdo_pps_apdo_min_voltage(snk));
> - port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
> -   pdo_pps_apdo_max_voltage(snk));
> - port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> - port->pps_data.out_volt 

Re: [PATCH] hwmon: (dell-smm) Add Dell Latitude 7440 to fan control whitelist

[Guenter Roeck]

On Tue, Mar 30, 2021 at 07:02:55PM +0200, Sebastian Oechsle wrote:
> Allow manual PWM control on Dell Latitude 7440.
> 
> Signed-off-by: Sebastian Oechsle  >

This patch is corrupt, to the point where it doesn't show up in hwmon patchwork.
I just happened to find it, but I can not convince git to apply it. Please
fix the problem and resubmit.

Thanks,
Guenter

> ---
> drivers/hwmon/dell-smm-hwmon.c | 8 
> 1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 73b9db9e3aab..2970892bed82 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -1210,6 +1210,14 @@ static struct dmi_system_id 
> i8k_whitelist_fan_control[] __initdata = {
>   },
>   .driver_data = (void *)_fan_control_data[I8K_FAN_34A3_35A3],
>   },
> + {
> + .ident = "Dell Latitude E7440",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Latitude E7440"),
> + },
> + .driver_data = (void *)_fan_control_data[I8K_FAN_34A3_35A3],
> + },
>   { }
> };
> 
> --
> 2.31.1

Re: [PATCH] hwmon: (dme1737): Add missing null check on return from platform_get_resource

[Guenter Roeck]

On Tue, Apr 06, 2021 at 07:54:58PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The call to platform_get_resource can potentially return a NULL pointer
> on failure, so add this check and return -EINVAL if it fails.
> 
> Addresses-Coverity: ("Dereference null return")
> Fixes: e95c237d78c0 ("hwmon: (dme1737) Add sch311x support")
> Signed-off-by: Colin Ian King 

Not really sure what to do with this; it is a false positive.
The resource is always set by the instantiating code (in the same
driver). Adding an error check just to make coverity happy seems
like a waste. Maybe we should introduce the equivalent of
devm_ioremap_resource() for IORESOURCE_IO.

Guenter

> ---
>  drivers/hwmon/dme1737.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hwmon/dme1737.c b/drivers/hwmon/dme1737.c
> index c1e4cfb40c3d..a2157872e126 100644
> --- a/drivers/hwmon/dme1737.c
> +++ b/drivers/hwmon/dme1737.c
> @@ -2633,6 +2633,8 @@ static int dme1737_isa_probe(struct platform_device 
> *pdev)
>   int err;
>  
>   res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + if (!res)
> + return -EINVAL;
>   if (!devm_request_region(dev, res->start, DME1737_EXTENT, "dme1737")) {
>   dev_err(dev, "Failed to request region 0x%04x-0x%04x.\n",
>   (unsigned short)res->start,

Re: [PATCH 5.11 00/45] 5.11.13-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:26AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.11.13 release.
> There are 45 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 

Build results:
total: 155 pass: 155 fail: 0
Qemu test results:
total: 460 pass: 460 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 5.10 00/41] 5.10.29-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:22AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.10.29 release.
> There are 41 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 

Build results:
total: 156 pass: 156 fail: 0
Qemu test results:
total: 454 pass: 454 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 5.4 00/23] 5.4.111-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:30AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.4.111 release.
> There are 23 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 

Build results:
total: 157 pass: 157 fail: 0
Qemu test results:
total: 433 pass: 433 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 4.19 00/18] 4.19.186-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:28AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.19.186 release.
> There are 18 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 

Build results:
total: 155 pass: 155 fail: 0
Qemu test results:
total: 423 pass: 423 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 4.14 00/14] 4.14.230-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:25AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.14.230 release.
> There are 14 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 

Build results:
total: 168 pass: 168 fail: 0
Qemu test results:
total: 408 pass: 408 fail: 0

Tested-by: Guenter Roeck 

Having said this, I did see a spurious crash, and I see an unusual warning.
I have seen the crash only once, but the warning happens with every boot.
These are likely not new but exposed because I added network interface
tests. This is all v4.14.y specific; I did not see it in other branches.
See below for the tracebacks. Maybe someone has seen it before.

Thanks,
Guenter

---
ftgmac100 1e66.ethernet eth0: NCSI interface down
[ cut here ]
WARNING: CPU: 0 PID: 477 at drivers/base/dma-mapping.c:325 
remap_allocator_free+0x54/0x5c
trying to free invalid coherent area: 909a1000
Modules linked in:
CPU: 0 PID: 477 Comm: ip Not tainted 4.14.230-rc1-00015-gbbc0ac1df344 #1
Hardware name: Generic DT based system
[<8000f8dc>] (unwind_backtrace) from [<8000d194>] (show_stack+0x10/0x14)
[<8000d194>] (show_stack) from [<805a5a80>] (__warn+0xc0/0xf4)
[<805a5a80>] (__warn) from [<800177b4>] (warn_slowpath_fmt+0x38/0x48)
[<800177b4>] (warn_slowpath_fmt) from [<80010554>] 
(remap_allocator_free+0x54/0x5c)
[<80010554>] (remap_allocator_free) from [<80010e4c>] 
(__arm_dma_free.constprop.0+0xec/0x13c)
[<80010e4c>] (__arm_dma_free.constprop.0) from [<80429924>] 
(ftgmac100_free_rings+0x17c/0x1f8)
[<80429924>] (ftgmac100_free_rings) from [<80429a24>] (ftgmac100_stop+0x84/0xa4)
[<80429a24>] (ftgmac100_stop) from [<804e8a70>] (__dev_close_many+0xac/0x100)
[<804e8a70>] (__dev_close_many) from [<804f0dc0>] 
(__dev_change_flags+0xb4/0x1a0)
[<804f0dc0>] (__dev_change_flags) from [<804f0ec4>] (dev_change_flags+0x18/0x48)
[<804f0ec4>] (dev_change_flags) from [<80561644>] (devinet_ioctl+0x6cc/0x808)
[<80561644>] (devinet_ioctl) from [<804d1548>] (sock_ioctl+0x188/0x2e4)
[<804d1548>] (sock_ioctl) from [<800eac80>] (do_vfs_ioctl+0x3a0/0x82c)
[<800eac80>] (do_vfs_ioctl) from [<800eb140>] (SyS_ioctl+0x34/0x60)
[<800eb140>] (SyS_ioctl) from [<8000a600>] (ret_fast_syscall+0x0/0x28)
---[ end trace c13f2f82f69274ad ]---

=

ftgmac100 1e66.ethernet eth0: NCSI interface up
Unable to handle kernel NULL pointer dereference at virtual address 
pgd = 9ec84000
[] *pgd=9f7f6831, *pte=, *ppte=
Internal error: Oops: 17 [#1] ARM
Modules linked in:
CPU: 0 PID: 397 Comm: default.script Not tainted 
4.14.230-rc1-00015-gbbc0ac1df344 #1
Hardware name: Generic DT based system
task: 9f5cc260 task.stack: 9ecee000
PC is at anon_vma_clone+0x64/0x19c
LR is at fs_reclaim_release+0x8/0x18
pc : [<800c1ccc>]lr : [<80098b5c>]psr: a153
sp : 9ecefe78  ip :   fp : 
r10: 01000200  r9 : 9f7e6d10  r8 : 80cb9a44
r7 : 9f7e0da0  r6 : 9f7e6d10  r5 : 9ed0f600  r4 : 9f5a562c
r3 : 0030  r2 : 9fbdf618  r1 : 0034  r0 : 9ed0f600
Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment user
Control: 00c5387d  Table: 9ec84008  DAC: 0055
Process default.script (pid: 397, stack limit = 0x9ecee188)
Stack: (0x9ecefe78 to 0x9ecf)
fe60:   9f5a303c 9f5a3000
fe80: 0002 9f7e0da0 9f7e0ab4 9f5a3000 9f77e600 9f7e0da0 9f5a3000 9f77e400
fea0: 9f72dc64 800c1e28 9f7e0ab0 9f7e0ab4 0002 9f77e600 9f7e0da0 800161f8
fec0: 9f5cc640 cacd966c 9f5cc260 cd397f94 80cb0afc  80016870 
fee0:  9f69f2f8 9f7e0aa8 807ca224 9f7e0aa0 9f72dc70 9f69f100 0011
ff00: 9f77e658 9f77e458 9eceff08 9eceff08 9f5cc650 0011 7eb26888 
ff20:   9ecee000  76eff3a0 80016870  
ff40:  7eb26888 9eceff78  9ecee000 76ec4a28 7eb26888 9eceff78
ff60:  7eb26888 0008  0008 800245e4 76efdcd0 7eb26888
ff80:  0002 8000a704 9ecee000  80016cd4  
ffa0: 9ecee000 8000a520 76efdcd0 7eb26888 76efffcc 0001 76efe7ac 
ffc0: 76efdcd0 7eb26888  0002 7eb26918 76efe000 76f00c60 76eff3a0
ffe0: 000e0350 7eb26888 76e96b94 76e96b98 6150 76efffcc  
[<800c1ccc>] (anon_vma_clone) from [<800c1e28>] (anon_vma_fork+0x24/0x138)
[<800c1e28>] (anon_vma_fork) from [<800161f8>] 
(copy_process.part.0+0x12a4/0x17dc)
[<800161f8>] (copy_process.part.0) from [<80016870>] (_do_fork+0xa0/0x488)
[<80016870>] (_do_fork) from [<80016cd4>] (sys_fork+0x24/0x2c)
[<80016cd4>] (sys_fork) from [<8000a520>] (ret_fast_syscall+0x0/0x4c)
Code: eb001f58 e2505000 0a17 e594b004 (e59b9000)
---[ end trace 6680cdd56c4514b7 ]---

Re: [PATCH 4.9 00/13] 4.9.266-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:20AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.9.266 release.
> There are 13 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 

Build results:
total: 163 pass: 163 fail: 0
Qemu test results:
total: 383 pass: 383 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH 4.4 00/20] 4.4.266-rc1 review

[Guenter Roeck]

On Fri, Apr 09, 2021 at 11:53:06AM +0200, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 4.4.266 release.
> There are 20 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 11 Apr 2021 09:52:52 +.
> Anything received after that time might be too late.
> 
Build results:
total: 160 pass: 160 fail: 0
Qemu test results:
total: 328 pass: 328 fail: 0

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH v1] watchdog: add new parameter to start the watchdog on module insertion

[Guenter Roeck]

On 4/9/21 2:34 AM, Flavio Suligoi wrote:
> The new parameter "start_enabled" starts the watchdog at the same time
> of the module insertion.
> This feature is very useful in embedded systems, to avoid cases where
> the system hangs before reaching userspace.
> 
> This function can be also enabled in the kernel config, so can be
> used when the watchdog driver is build as built-in.
> 
> This parameter involves the "core" section of the watchdog driver;
> in this way it is common for all the watchdog hardware implementations.
> 
> Note: to use only for watchdog drivers which doesn't support this
>   parameter by itself.
> 
> Signed-off-by: Flavio Suligoi 
> ---
>  Documentation/watchdog/watchdog-parameters.rst |  5 +
>  drivers/watchdog/Kconfig   | 14 ++
>  drivers/watchdog/watchdog_core.c   | 12 
>  3 files changed, 31 insertions(+)
> 
> diff --git a/Documentation/watchdog/watchdog-parameters.rst 
> b/Documentation/watchdog/watchdog-parameters.rst
> index 223c99361a30..623fd064df91 100644
> --- a/Documentation/watchdog/watchdog-parameters.rst
> +++ b/Documentation/watchdog/watchdog-parameters.rst
> @@ -21,6 +21,11 @@ watchdog core:
>   timeout. Setting this to a non-zero value can be useful to ensure that
>   either userspace comes up properly, or the board gets reset and allows
>   fallback logic in the bootloader to try something else.
> +start_enabled:
> + Watchdog is started on module insertion. This option can be also
> + selected by kernel config (default=kernel config parameter).
> + Use only for watchdog drivers which doesn't support this parameter
> + by itself.

Why ?

>  
>  -
>  
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0470dc15c085..c2a668d6bbbc 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -47,6 +47,20 @@ config WATCHDOG_NOWAYOUT
> get killed. If you say Y here, the watchdog cannot be stopped once
> it has been started.
>  
> +config WATCHDOG_START_ENABLED
> + bool "Start watchdog on module insertion"
> + help
> +   Say Y if you want to start the watchdog at the same time when the
> +   driver is loaded.
> +   This feature is very useful in embedded systems, to avoid cases where
> +   the system could hang before reaching userspace.
> +   This parameter involves the "core" section of the watchdog driver,
> +   in this way it is common for all the watchdog hardware
> +   implementations.

"This parameter applies to all watchdog drivers.". The rest is implementation
detail and irrelevant here.

> +
> +   Note: to use only for watchdog drivers which doesn't support this
> + parameter by itself.
> +

This comment is quite useless in the Kconfig description. If enabled, it is 
enabled,
period.

>  config WATCHDOG_HANDLE_BOOT_ENABLED
>   bool "Update boot-enabled watchdog until userspace takes over"
>   default y
> diff --git a/drivers/watchdog/watchdog_core.c 
> b/drivers/watchdog/watchdog_core.c
> index 5df0a22e2cb4..5052ae355219 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -43,6 +43,11 @@ static int stop_on_reboot = -1;
>  module_param(stop_on_reboot, int, 0444);
>  MODULE_PARM_DESC(stop_on_reboot, "Stop watchdogs on reboot (0=keep watching, 
> 1=stop)");
>  
> +static bool start_enabled = IS_ENABLED(CONFIG_WATCHDOG_START_ENABLED);
> +module_param(start_enabled, bool, 0444);
> +MODULE_PARM_DESC(start_enabled, "Start watchdog on module insertion 
> (default="
> + __MODULE_STRING(IS_ENABLED(CONFIG_WATCHDOG_START_ENABLED)) ")");
> +
>  /*
>   * Deferred Registration infrastructure.
>   *
> @@ -224,6 +229,13 @@ static int __watchdog_register_device(struct 
> watchdog_device *wdd)
>* corrupted in a later stage then we expect a kernel panic!
>*/
>  
> + /* If required, start the watchdog immediately */
> + if (start_enabled) {
> + set_bit(WDOG_HW_RUNNING, >status);
> + wdd->ops->start(wdd);
> + pr_info("Watchdog enabled\n");
> + }
> +
>   /* Use alias for watchdog id if possible */
>   if (wdd->parent) {
>   ret = of_alias_get_id(wdd->parent->of_node, "watchdog");
> 

Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available

[Guenter Roeck]

On 4/9/21 2:55 AM, Wang Qing wrote:
> Use the bark interrupt as the pretimeout notifier if available.
> 
> By default, the pretimeout notification shall occur one second earlier
> than the timeout.
> 
> Signed-off-by: Wang Qing 
> ---
>  drivers/watchdog/mtk_wdt.c | 47 
> +++---
>  1 file changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 97ca993..8b919cc
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define WDT_MAX_TIMEOUT  31
>  #define WDT_MIN_TIMEOUT  1
> @@ -234,18 +235,35 @@ static int mtk_wdt_start(struct watchdog_device 
> *wdt_dev)
>   void __iomem *wdt_base = mtk_wdt->wdt_base;
>   int ret;
>  
> - ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> + ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - 
> wdt_dev->pretimeout);

That looks suspiciously like the real watchdog won't happen at all.
What will happen if the pretimeout governor is set to none ?

Guenter

>   if (ret < 0)
>   return ret;
>  
>   reg = ioread32(wdt_base + WDT_MODE);
>   reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
> + if (wdt_dev->pretimeout)
> + reg |= WDT_MODE_IRQ_EN;
>   reg |= (WDT_MODE_EN | WDT_MODE_KEY);
>   iowrite32(reg, wdt_base + WDT_MODE);
>  
>   return 0;
>  }
>  
> +static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd,
> +unsigned int timeout)
> +{
> + wdd->pretimeout = timeout;
> + return mtk_wdt_start(wdd);
> +}
> +
> +static irqreturn_t mtk_wdt_isr(int irq, void *arg)
> +{
> + struct watchdog_device *wdd = arg;
> + watchdog_notify_pretimeout(wdd);
> +
> + return IRQ_HANDLED;
> +}
> +
>  static const struct watchdog_info mtk_wdt_info = {
>   .identity   = DRV_NAME,
>   .options= WDIOF_SETTIMEOUT |
> @@ -253,12 +271,21 @@ static const struct watchdog_info mtk_wdt_info = {
> WDIOF_MAGICCLOSE,
>  };
>  
> +static const struct watchdog_info mtk_wdt_pt_info = {
> + .identity   = DRV_NAME,
> + .options= WDIOF_SETTIMEOUT |
> +   WDIOF_PRETIMEOUT |
> +   WDIOF_KEEPALIVEPING |
> +   WDIOF_MAGICCLOSE,
> +};
> +
>  static const struct watchdog_ops mtk_wdt_ops = {
>   .owner  = THIS_MODULE,
>   .start  = mtk_wdt_start,
>   .stop   = mtk_wdt_stop,
>   .ping   = mtk_wdt_ping,
>   .set_timeout= mtk_wdt_set_timeout,
> + .set_pretimeout = mtk_wdt_set_pretimeout,
>   .restart= mtk_wdt_restart,
>  };
>  
> @@ -267,7 +294,7 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   struct device *dev = >dev;
>   struct mtk_wdt_dev *mtk_wdt;
>   const struct mtk_wdt_data *wdt_data;
> - int err;
> + int err,irq;
>  
>   mtk_wdt = devm_kzalloc(dev, sizeof(*mtk_wdt), GFP_KERNEL);
>   if (!mtk_wdt)
> @@ -279,7 +306,21 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   if (IS_ERR(mtk_wdt->wdt_base))
>   return PTR_ERR(mtk_wdt->wdt_base);
>  
> - mtk_wdt->wdt_dev.info = _wdt_info;
> + irq = platform_get_irq(pdev, 0);
> + if (irq > 0) {
> + err = devm_request_irq(>dev, irq, mtk_wdt_isr, 0, 
> "wdt_bark", _wdt->wdt_dev);
> + if (err)
> + return err;
> +
> + mtk_wdt->wdt_dev.info = _wdt_pt_info;
> + mtk_wdt->wdt_dev.pretimeout = 1;
> + } else {
> + if (irq == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + mtk_wdt->wdt_dev.info = _wdt_info;
> + }
> +
>   mtk_wdt->wdt_dev.ops = _wdt_ops;
>   mtk_wdt->wdt_dev.timeout = WDT_MAX_TIMEOUT;
>   mtk_wdt->wdt_dev.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT * 1000;
> 

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 3:53 PM, Frank Rowand wrote:
> On 4/8/21 4:54 PM, Guenter Roeck wrote:
>> On 4/8/21 2:28 PM, Rob Herring wrote:
>>>
>>> Applying now so this gets into linux-next this week.
>>>
>> The patch doesn't apply on top of today's -next; it conflicts
>> with "of: properly check for error returned by fdt_get_name()".
>>
>> I reverted that patch and applied this one, and the DT unittests
>> run with it on openrisc. I do get a single test failure, but I that
>> is a different problem (possibly with the test case itself).
>>
>> ### dt-test ### FAIL of_unittest_dma_ranges_one():923 of_dma_get_range: 
>> wrong DMA addr 0x
>>  (expecting 1) on node 
>> /testcase-data/address-tests/bus@8000/device@1000
> 
> That is a known regression on the target that I use for testing (and
> has been since 5.10-rc1) - the 8074 dragonboard, arm 32.  No
> one else has reported it on the list, so even though I want to debug
> and fix it "promptly", other tasks have had higher priority.  In my
> notes I list two suspect commits:
> 
>   e0d072782c73 dma-mapping: introduce DMA range map, supplanting 
> dma_pfn_offset
>   0a0f0d8be76d dma-mapping: split 
> 
> I think that was purely based on looking at the list of commits that
> may have touched OF dma.  I have not done a bisect.
> 

Here you are:

# bad: [2c85ebc57b3e1817b6ce1a6b703928e113a90442] Linux 5.10
# good: [bbf5c979011a099af5dc76498918ed7df445635b] Linux 5.9
git bisect start 'v5.10' 'v5.9'
# bad: [4d0e9df5e43dba52d38b251e3b909df8fa1110be] lib, uaccess: add failure 
injection to usercopy functions
git bisect bad 4d0e9df5e43dba52d38b251e3b909df8fa1110be
# good: [f888bdf9823c85fe945c4eb3ba353f749dec3856] Merge tag 
'devicetree-for-5.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
git bisect good f888bdf9823c85fe945c4eb3ba353f749dec3856
# good: [640eee067d9aae0bb98d8706001976ff1affaf00] Merge tag 
'drm-misc-next-fixes-2020-10-13' of git://anongit.freedesktop.org/drm/drm-misc 
into drm-next
git bisect good 640eee067d9aae0bb98d8706001976ff1affaf00
# good: [c6dbef7307629cce855aa6b482b60cbfed88] Merge tag 'usb-5.10-rc1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect good c6dbef7307629cce855aa6b482b60cbfed88
# good: [ce1558c285f9ad04c03b46833a028230771cc0a7] ALSA: hda/hdmi: fix 
incorrect locking in hdmi_pcm_close
git bisect good ce1558c285f9ad04c03b46833a028230771cc0a7
# good: [c48b75b7271db23c1b2d1204d6e8496d91f27711] Merge tag 'sound-5.10-rc1' 
of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good c48b75b7271db23c1b2d1204d6e8496d91f27711
# bad: [0cd7d9795fa82226e7516d38b474bddae8b1ff26] Merge branch 'for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching
git bisect bad 0cd7d9795fa82226e7516d38b474bddae8b1ff26
# good: [b1839e7c2a42ccd9a0587c0092e880c7a213ee2a] dmaengine: xilinx: dpdma: 
convert tasklets to use new tasklet_setup() API
git bisect good b1839e7c2a42ccd9a0587c0092e880c7a213ee2a
# bad: [0de327969b61a245e3a47b60009eae73fe513cef] cma: decrease CMA_ALIGNMENT 
lower limit to 2
git bisect bad 0de327969b61a245e3a47b60009eae73fe513cef
# good: [6eb0233ec2d0df288fe8515d5b0b2b15562e05bb] usb: don't inherity DMA 
properties for USB devices
git bisect good 6eb0233ec2d0df288fe8515d5b0b2b15562e05bb
# bad: [48d15814dd0fc429e3205b87f1af6cc472018478] lib82596: move DMA allocation 
into the callers of i82596_probe
git bisect bad 48d15814dd0fc429e3205b87f1af6cc472018478
# bad: [eba304c6861613a649ba46cfab835b1eddeacd8e] dma-mapping: better document 
dma_addr_t and DMA_MAPPING_ERROR
git bisect bad eba304c6861613a649ba46cfab835b1eddeacd8e
# bad: [b9bb694b9f62f4b31652223ed3ca38cf98bbb370] iommu/io-pgtable-arm: Clean 
up faulty sanity check
git bisect bad b9bb694b9f62f4b31652223ed3ca38cf98bbb370
# bad: [a97740f81874c8063c12c24f34d25f10c4f5e9aa] dma-debug: convert comma to 
semicolon
git bisect bad a97740f81874c8063c12c24f34d25f10c4f5e9aa
# bad: [e0d072782c734d27f5af062c62266f2598f68542] dma-mapping: introduce DMA 
range map, supplanting dma_pfn_offset
git bisect bad e0d072782c734d27f5af062c62266f2598f68542
# first bad commit: [e0d072782c734d27f5af062c62266f2598f68542] dma-mapping: 
introduce DMA range map, supplanting dma_pfn_offset

Guenter

Re: [PATCH v3] hwmon: Add driver for fsp-3y PSUs and PDUs

[Guenter Roeck]

On 4/8/21 6:27 PM, Václav Kubernát wrote:
> This patch adds support for these devices:
> - YH-5151E - the PDU
> - YM-2151E - the PSU
> 
> The device datasheet says that the devices support PMBus 1.2, but in my
> testing, a lot of the commands aren't supported and if they are, they
> sometimes behave strangely or inconsistently. For example, writes to the
> PAGE command requires using PEC, otherwise the write won't work and the
> page won't switch, even though, the standard says that PEC is opiotnal.
> On the other hand, writes the SMBALERT don't require PEC. Because of
> this, the driver is mostly reverse engineered with the help of a tool
> called pmbus_peek written by David Brownell (and later adopted by my
> colleague Jan Kundrát).
> 
> The device also has some sort of a timing issue when switching pages,
> which is explained further in the code.
> 
> Because of this, the driver support is limited. It exposes only the
> values, that have been tested to work correctly.
> 
> Signed-off-by: Václav Kubernát 
> ---
>  Documentation/hwmon/fsp-3y.rst |  26 
>  drivers/hwmon/pmbus/Kconfig|  10 ++
>  drivers/hwmon/pmbus/Makefile   |   1 +
>  drivers/hwmon/pmbus/fsp-3y.c   | 236 +
>  4 files changed, 273 insertions(+)
>  create mode 100644 Documentation/hwmon/fsp-3y.rst
>  create mode 100644 drivers/hwmon/pmbus/fsp-3y.c
> 
> diff --git a/Documentation/hwmon/fsp-3y.rst b/Documentation/hwmon/fsp-3y.rst
> new file mode 100644
> index ..68a547021846
> --- /dev/null
> +++ b/Documentation/hwmon/fsp-3y.rst
> @@ -0,0 +1,26 @@
> +Kernel driver fsp3y
> +==
> +Supported devices:
> +  * 3Y POWER YH-5151E
> +  * 3Y POWER YM-2151E
> +
> +Author: Václav Kubernát 
> +
> +Description
> +---
> +This driver implements limited support for two 3Y POWER devices.
> +
> +Sysfs entries
> +-
> +in1_inputinput voltage
> +in2_input12V output voltage
> +in3_input5V output voltage
> +curr1_input  input current
> +curr2_input  12V output current
> +curr3_input  5V output current
> +fan1_input   fan rpm
> +temp1_input  temperature 1
> +temp2_input  temperature 2
> +temp3_input  temperature 3
> +power1_input input power
> +power2_input output power
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 03606d4298a4..9d12d446396c 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -56,6 +56,16 @@ config SENSORS_BEL_PFE
> This driver can also be built as a module. If so, the module will
> be called bel-pfe.
>  
> +config SENSORS_FSP_3Y
> + tristate "FSP/3Y-Power power supplies"
> + help
> +   If you say yes here you get hardware monitoring support for
> +   FSP/3Y-Power hot-swap power supplies.
> +   Supported models: YH-5151E, YM-2151E
> +
> +   This driver can also be built as a module. If so, the module will
> +   be called fsp-3y.
> +
>  config SENSORS_IBM_CFFPS
>   tristate "IBM Common Form Factor Power Supply"
>   depends on LEDS_CLASS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 6a4ba0fdc1db..bfe218ad898f 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS)   += pmbus.o
>  obj-$(CONFIG_SENSORS_ADM1266)+= adm1266.o
>  obj-$(CONFIG_SENSORS_ADM1275)+= adm1275.o
>  obj-$(CONFIG_SENSORS_BEL_PFE)+= bel-pfe.o
> +obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
>  obj-$(CONFIG_SENSORS_IBM_CFFPS)  += ibm-cffps.o
>  obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
>  obj-$(CONFIG_SENSORS_IR35221)+= ir35221.o
> diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c
> new file mode 100644
> index ..f03c4e27ec8c
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/fsp-3y.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for FSP 3Y-Power PSUs
> + *
> + * Copyright (c) 2021 Václav Kubernát, CESNET
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "pmbus.h"
> +
> +#define YM2151_PAGE_12V_LOG  0x00
> +#define YM2151_PAGE_12V_REAL 0x00
> +#define YM2151_PAGE_5VSB_LOG 0x01
> +#define YM2151_PAGE_5VSB_REAL0x20
> +#define YH5151E_PAGE_12V_LOG 0x00
> +#define YH5151E_PAGE_12V_REAL0x00
> +#define YH5151E_PAGE_5V_LOG  0x01
> +#define YH5151E_PAGE_5V_REAL 0x10
> +#define YH5151E_PAGE_3V3_LOG 0x02
> +#define YH5151E_PAGE_3V3_REAL0x11
> +
> +enum chips {
> + ym2151e,
> + yh5151e
> +};
> +
> +struct fsp3y_data {
> + struct pmbus_driver_info info;
> + enum chips chip;
> + int page;
> +};
> +
> +#define to_fsp3y_data(x) container_of(x, struct fsp3y_data, info)
> +
> +static int page_log_to_page_real(int page_log, enum chips chip)
> +{
> + switch (chip) {
> + case 

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 2:28 PM, Rob Herring wrote:
> 
> Applying now so this gets into linux-next this week.
> 
The patch doesn't apply on top of today's -next; it conflicts
with "of: properly check for error returned by fdt_get_name()".

I reverted that patch and applied this one, and the DT unittests
run with it on openrisc. I do get a single test failure, but I that
is a different problem (possibly with the test case itself).

### dt-test ### FAIL of_unittest_dma_ranges_one():923 of_dma_get_range: wrong 
DMA addr 0x
(expecting 1) on node 
/testcase-data/address-tests/bus@8000/device@1000

Tested-by: Guenter Roeck 

Guenter

Re: [PATCH v3 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 1:06 PM, Frank Rowand wrote:

>>> +#define FDT_ALIGN_SIZE 8
>>> +
>>
>> Use existing define ? Or was that local in libfdt ?
> 
> I don't see a define in libfdt.  If anyone finds one,
> I'll switch to it.
> 

Turns out that was hardcoded in scripts/dtc/libfdt/fdt.c

+   /* The device tree must be at an 8-byte aligned address */
+   if ((uintptr_t)fdt & 7)
+   return -FDT_ERR_ALIGNMENT;
+

Guenter

Re: [PATCH] block: Disable -Walign-mismatch for blk-mq.c

[Guenter Roeck]

On 4/8/21 12:44 PM, Nathan Chancellor wrote:
> LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in
> blk_mq_complete_send_ipi():
> 
> block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to
> 32-byte aligned parameter 2 of 'smp_call_function_single_async' may
> result in an unaligned pointer access [-Walign-mismatch]
> smp_call_function_single_async(cpu, >csd);
> ^
> 1 warning generated.
> 
> This is expected after commit 4ccafe032005 ("block: unalign
> call_single_data in struct request"), which purposefully unaligned the
> structure to save space. Given that there is no real alignment
> requirement and there have been no reports of issues since that change,
> it should be safe to disable the warning for this one translation unit.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1328
> Link: 
> https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/
> Link: https://lore.kernel.org/r/20210330230249.709221-1-jian...@google.com/
> Signed-off-by: Nathan Chancellor 

Reviewed-by: Guenter Roeck 

> ---
>  block/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/Makefile b/block/Makefile
> index 8d841f5f986f..d69ac0bd8e61 100644
> --- a/block/Makefile
> +++ b/block/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o 
> blk-sysfs.o \
>   blk-mq-sysfs.o blk-mq-cpumap.o blk-mq-sched.o ioctl.o \
>   genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o
>  
> +CFLAGS_blk-mq.o := $(call cc-disable-warning, align-mismatch)
>  obj-$(CONFIG_BOUNCE) += bounce.o
>  obj-$(CONFIG_BLK_SCSI_REQUEST)   += scsi_ioctl.o
>  obj-$(CONFIG_BLK_DEV_BSG)+= bsg.o
> 
> base-commit: e49d033bddf5b565044e2abe4241353959bc9120
> 

Re: [PATCH v3 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 8:17 AM, frowand.l...@gmail.com wrote:
> From: Frank Rowand 
> 
> The Devicetree standard specifies an 8 byte alignment of the FDT.
> Code in libfdt expects this alignment for an FDT image in memory.
> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
> with kmalloc(), align pointer, memcpy() to get proper alignment.
> 
> The 4 byte alignment exposed a related bug which triggered a crash
> on openrisc with:
> commit 79edff12060f ("scripts/dtc: Update to upstream version 
> v1.6.0-51-g183df9e9c2b9")
> as reported in:
> https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/
> 
> Reported-by: Guenter Roeck 
> Signed-off-by: Frank Rowand 
> ---
> 
> changes since version 1:
>   - use pointer from kmalloc() for kfree() instead of using pointer that
> has been modified for FDT alignment
> 
> changes since version 2:
>   - version 1 was a work in progress version, I failed to commit the following
> final changes
>   - reorder first two arguments of of_overlay_apply()
> 
>  drivers/of/of_private.h |  2 ++
>  drivers/of/overlay.c| 28 +---
>  drivers/of/unittest.c   | 12 +---
>  3 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index d9e6a324de0a..d717efbd637d 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -8,6 +8,8 @@
>   * Copyright (C) 1996-2005 Paul Mackerras.
>   */
>  
> +#define FDT_ALIGN_SIZE 8
> +

Use existing define ? Or was that local in libfdt ?

>  /**
>   * struct alias_prop - Alias property in 'aliases' node
>   * @link:List node to link the structure in aliases_lookup list
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 50bbe0edf538..cf770452e1e5 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -57,7 +57,7 @@ struct fragment {
>   * struct overlay_changeset
>   * @id:  changeset identifier
>   * @ovcs_list:   list on which we are located
> - * @fdt: FDT that was unflattened to create @overlay_tree
> + * @fdt: base of memory allocated to hold aligned FDT that was 
> unflattened to create @overlay_tree
>   * @overlay_tree:expanded device tree that contains the fragment nodes
>   * @count:   count of fragment structures
>   * @fragments:   fragment nodes in the overlay expanded device 
> tree
> @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node 
> *info_node)
>  /**
>   * init_overlay_changeset() - initialize overlay changeset from overlay tree
>   * @ovcs:Overlay changeset to build
> - * @fdt: the FDT that was unflattened to create @tree
> - * @tree:Contains all the overlay fragments and overlay fixup nodes
> + * @fdt: base of memory allocated to hold aligned FDT that was 
> unflattened to create @tree
> + * @tree:Contains the overlay fragments and overlay fixup nodes
>   *
>   * Initialize @ovcs.  Populate @ovcs->fragments with node information from
>   * the top level of @tree.  The relevant top level nodes are the fragment
> @@ -873,7 +873,8 @@ static void free_overlay_changeset(struct 
> overlay_changeset *ovcs)
>   * internal documentation
>   *
>   * of_overlay_apply() - Create and apply an overlay changeset
> - * @fdt: the FDT that was unflattened to create @tree
> + * @fdt: base of memory allocated to hold *@fdt_align
> + * @fdt_align:   the FDT that was unflattened to create @tree, aligned
>   * @tree:Expanded overlay device tree
>   * @ovcs_id: Pointer to overlay changeset id
>   *
> @@ -912,8 +913,8 @@ static void free_overlay_changeset(struct 
> overlay_changeset *ovcs)
>   * id is returned to *ovcs_id.
>   */
>  
> -static int of_overlay_apply(const void *fdt, struct device_node *tree,
> - int *ovcs_id)
> +static int of_overlay_apply(const void *fdt, const void *fdt_align,

fdt_align is not used in this function.

> + struct device_node *tree, int *ovcs_id)
>  {
>   struct overlay_changeset *ovcs;
>   int ret = 0, ret_revert, ret_tmp;
> @@ -953,7 +954,7 @@ static int of_overlay_apply(const void *fdt, struct 
> device_node *tree,
>   /*
>* after overlay_notify(), ovcs->overlay_tree related pointers may have
>* leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree;
> -  * and can not free fdt, aka ovcs->fdt
> +  * and can not free memory containing aligned fdt, aka ovcs->fdt

ovcs->fdt does not point to the aligned fdt, but to the allocated fdt.

>*/
>   ret = overlay_notify(ovcs, OF_OVERLAY_P

Re: [PATCH v2 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 7:43 AM, frowand.l...@gmail.com wrote:
> From: Frank Rowand 
> 
> The Devicetree standard specifies an 8 byte alignment of the FDT.
> Code in libfdt expects this alignment for an FDT image in memory.
> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
> with kmalloc(), align pointer, memcpy() to get proper alignment.
> 
> The 4 byte alignment exposed a related bug which triggered a crash
> on openrisc with:
> commit 79edff12060f ("scripts/dtc: Update to upstream version 
> v1.6.0-51-g183df9e9c2b9")
> as reported in:
> https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/
> 
> Reported-by: Guenter Roeck 
> Signed-off-by: Frank Rowand 
> 
> ---
> 
> Please review carefully, I am not yet fully awake...
> 
> changes since version 1:
>   - use pointer from kmalloc() for kfree() instead of using pointer that
> has been modified for FDT alignment
> 
>  drivers/of/of_private.h |  2 ++
>  drivers/of/overlay.c| 28 +---
>  drivers/of/unittest.c   | 12 +---
>  3 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index d9e6a324de0a..d717efbd637d 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -8,6 +8,8 @@
>   * Copyright (C) 1996-2005 Paul Mackerras.
>   */
>  
> +#define FDT_ALIGN_SIZE 8
> +

Wasn't there a define for that elsewhere ?

>  /**
>   * struct alias_prop - Alias property in 'aliases' node
>   * @link:List node to link the structure in aliases_lookup list
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 50bbe0edf538..e0397d70d531 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -57,7 +57,7 @@ struct fragment {
>   * struct overlay_changeset
>   * @id:  changeset identifier
>   * @ovcs_list:   list on which we are located
> - * @fdt: FDT that was unflattened to create @overlay_tree
> + * @fdt: base of memory allocated to hold aligned FDT that was 
> unflattened to create @overlay_tree
>   * @overlay_tree:expanded device tree that contains the fragment nodes
>   * @count:   count of fragment structures
>   * @fragments:   fragment nodes in the overlay expanded device 
> tree
> @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node 
> *info_node)
>  /**
>   * init_overlay_changeset() - initialize overlay changeset from overlay tree
>   * @ovcs:Overlay changeset to build
> - * @fdt: the FDT that was unflattened to create @tree
> - * @tree:Contains all the overlay fragments and overlay fixup nodes
> + * @fdt: base of memory allocated to hold aligned FDT that was 
> unflattened to create @tree
> + * @tree:Contains the overlay fragments and overlay fixup nodes
>   *
>   * Initialize @ovcs.  Populate @ovcs->fragments with node information from
>   * the top level of @tree.  The relevant top level nodes are the fragment
> @@ -873,7 +873,8 @@ static void free_overlay_changeset(struct 
> overlay_changeset *ovcs)
>   * internal documentation
>   *
>   * of_overlay_apply() - Create and apply an overlay changeset
> - * @fdt: the FDT that was unflattened to create @tree
> + * @fdt_align:   the FDT that was unflattened to create @tree, aligned
> + * @fdt: base of memory allocated to hold *@fdt_align
>   * @tree:Expanded overlay device tree
>   * @ovcs_id: Pointer to overlay changeset id
>   *
> @@ -912,8 +913,8 @@ static void free_overlay_changeset(struct 
> overlay_changeset *ovcs)
>   * id is returned to *ovcs_id.
>   */
>  
> -static int of_overlay_apply(const void *fdt, struct device_node *tree,
> - int *ovcs_id)
> +static int of_overlay_apply(const void *fdt_align, const void *fdt,
> + struct device_node *tree, int *ovcs_id)

Is fdt_align used anywhere in this function ?

On a side note, it seems messy that of_overlay_apply() calls kfree on error.
That would probably be better handled in the calling code.

>  {
>   struct overlay_changeset *ovcs;
>   int ret = 0, ret_revert, ret_tmp;
> @@ -953,7 +954,7 @@ static int of_overlay_apply(const void *fdt, struct 
> device_node *tree,
>   /*
>* after overlay_notify(), ovcs->overlay_tree related pointers may have
>* leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree;
> -  * and can not free fdt, aka ovcs->fdt
> +  * and can not free memory containing aligned fdt, aka ovcs->fdt

fdt doesn't point to the aligned fdt, though. ovcs->fdt is the allocated fdt.

>*/
>   ret = overlay_notify(ovcs, O

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On Mon, Apr 05, 2021 at 10:48:10PM +0200, Thomas Weißschuh wrote:
> Changes since v1:
> * Incorporate feedback from Barnabás Pőcze
>   * Use a WMI driver instead of a platform driver
>   * Let the kernel manage the driver lifecycle
>   * Fix errno/ACPI error confusion
>   * Fix resource cleanup
>   * Document reason for integer casting
> 
> Thank you Barnabás for your review, it is much appreciated.
> 
> -- >8 --
> 
> Tested with a X570 I Aorus Pro Wifi.
> The mainboard contains an ITE IT8688E chip for management.
> This chips is also handled by drivers/hwmon/i87.c but as it is also used
> by the firmware itself it needs an ACPI driver.
> 
> Unfortunately not all sensor registers are handled by the firmware and even
> less are exposed via WMI.
> 
> Signed-off-by: Thomas Weißschuh 
> ---
>  drivers/platform/x86/Kconfig|  11 +++
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/gigabyte-wmi.c | 138 

Originally drivers/platform was supposed to be used for platform specific
code. Not that I have control over it, but I really dislike that more and
more hwmon drivers end up there.

At least hwmon is in good company - I see drivers for various other subsystems
there as well. I just wonder if that is such a good idea. That entire directory
is bypassing subsystem maintainer reviews.

Guenter

>  3 files changed, 150 insertions(+)
>  create mode 100644 drivers/platform/x86/gigabyte-wmi.c
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index ad4e630e73e2..96622a2106f7 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -123,6 +123,17 @@ config XIAOMI_WMI
> To compile this driver as a module, choose M here: the module will
> be called xiaomi-wmi.
>  
> +config GIGABYTE_WMI
> + tristate "Gigabyte WMI temperature driver"
> + depends on ACPI_WMI
> + depends on HWMON
> + help
> +   Say Y here if you want to support WMI-based temperature reporting on
> +   Gigabyte mainboards.
> +
> +   To compile this driver as a module, choose M here: the module will
> +   be called gigabyte-wmi.
> +
>  config ACERHDF
>   tristate "Acer Aspire One temperature and fan driver"
>   depends on ACPI && THERMAL
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 60d554073749..1621ebfd04fd 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += 
> intel-wmi-thunderbolt.o
>  obj-$(CONFIG_MXM_WMI)+= mxm-wmi.o
>  obj-$(CONFIG_PEAQ_WMI)   += peaq-wmi.o
>  obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> +obj-$(CONFIG_GIGABYTE_WMI)   += gigabyte-wmi.o
>  
>  # Acer
>  obj-$(CONFIG_ACERHDF)+= acerhdf.o
> diff --git a/drivers/platform/x86/gigabyte-wmi.c 
> b/drivers/platform/x86/gigabyte-wmi.c
> new file mode 100644
> index ..8618363e3ccf
> --- /dev/null
> +++ b/drivers/platform/x86/gigabyte-wmi.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *  Copyright (C) 2021 Thomas Weißschuh 
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define GIGABYTE_WMI_GUID "DEADBEEF-2001--00A0-C9062910"
> +
> +enum gigabyte_wmi_commandtype {
> + GIGABYTE_WMI_BUILD_DATE_QUERY   =   0x1,
> + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY   =   0x2,
> + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY =   0x4,
> + GIGABYTE_WMI_MAINBOARD_NAME_QUERY   =   0x5,
> + GIGABYTE_WMI_TEMPERATURE_QUERY  = 0x125,
> +};
> +
> +struct gigabyte_wmi_args {
> + u32 arg1;
> +};
> +
> +static int gigabyte_wmi_perform_query(enum gigabyte_wmi_commandtype command,
> + struct gigabyte_wmi_args *args, struct acpi_buffer *out)
> +{
> + const struct acpi_buffer in = {
> + .length = sizeof(*args),
> + .pointer = args,
> + };
> +
> + acpi_status ret = wmi_evaluate_method(GIGABYTE_WMI_GUID, 0x0, command, 
> , out);
> + if (ret == AE_OK) {
> + return 0;
> + } else {
> + return -EIO;
> + };
> +}
> +
> +static int gigabyte_wmi_query_integer(enum gigabyte_wmi_commandtype command,
> + struct gigabyte_wmi_args *args, u64 *res)
> +{
> + union acpi_object *obj;
> + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL };
> + int ret;
> +
> + ret = gigabyte_wmi_perform_query(command, args, );
> + if (ret) {
> + goto out;
> + }
> + obj = result.pointer;
> + if (obj && obj->type == ACPI_TYPE_INTEGER) {
> + *res = obj->integer.value;
> + ret = 0;
> + } else {
> + ret = -EIO;
> + }
> +out:
> + kfree(result.pointer);
> + return ret;
> +}
> +
> +static int gigabyte_wmi_temperature(u8 sensor, long *res)
> +{
> + struct 

Re: [PATCH v2] platform/x86: add Gigabyte WMI temperature driver

[Guenter Roeck]

On 4/8/21 2:36 AM, Hans de Goede wrote:
> Hi,
> 
> On 4/7/21 9:43 PM, Thomas Weißschuh wrote:
>> Hi Hans,
>>
>> On Mi, 2021-04-07T17:54+0200, Hans de Goede wrote:
>>> Thank you for your new driver and thank you for the quick respin
>>> addressing Barnabás' request to make it a WMI driver.
>>>
>>> The code looks good, so merging this should be a no-brainer,
>>> yet I'm not sure if I should merge this driver as-is, let me
>>> explain.
>>
>> thanks for the encouraging words.
>>
>>> The problem is that I assume that this is based on reverse-engineering?
>>
>> Yes, it is completely reverse-engineered.
>> Essentially I stumbled upon Matthews comment at
>> https://bugzilla.kernel.org/show_bug.cgi?id=204807#c37 and went from there.
>>
>>> We have some mixes experiences with reverse-engineered WMI drivers,
>>> sometimes a feature is not supported yet the wmi_evaluate_method()
>>> call happily succeeds. One example of this causing trouble is:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1797d588af15174d4a4e7159dac8c800538e4f8c
>>
>> There actually are reports of recent, similar mainboards with recent 
>> firmware and
>> similar sensor chips that do not support the temperature query.
>> (https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/3 and
>> https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/2)
>>
>> Unfortunately for unknown WMI queries the firmware does not return any value.
>> This ends up as an ACPI integer with value 0 on the driver side.
>> (Which I could not yet find documentation for if that is expected)
>> In the current version of the driver EIO is returned for 0 values which
>> get translated to N/A by lm-sensors.
>>
>>> At a minimum I think your driver should check in its
>>> probe function that
>>>
>>> gigabyte_wmi_query_integer(GIGABYTE_WMI_TEMPERATURE_QUERY, ...)
>>>
>>> actually succeeds on the machine the driver is running on chances
>>> are that Gigabyte has been using the DEADBEEF-2001--00A0-C9062910
>>> GUID for ages, where as the 0x125 value for GIGABYTE_WMI_TEMPERATURE_QUERY
>>> suggests that this is a pretty new API.
>>
>> Would it be enough to probe all six sensors and check if all return 0?
> 
> I think that failing the probe with -ENODEV, with a dev_info() explaining why 
> when
> all six sensors return 0 would be good yes, that should also fix those 2
> issues on https://github.com/t-8ch/linux-gigabyte-wmi-driver/.
> 
>>> It would be good if you can see if you can find some DSDT-s for older
>>> gigabyte motherboards attached to various distro's bug-tracking systems
>>> or forum posts and see how those respond to an unknown 
>>> gigabyte_wmi_commandtype.
>>
>> Will do.
> 
> Since you alreayd have bugreports of boards where this does not work,
> please don't spend too much time on this. I guess those older DSDT-s will
> also just return an integer with value 0.
> 
>>> Another open question to make sure this driver is suitable
>>> as a generic driver (and does not misbehave) is how to figure out
>>> how many temperature sensors there actually are.
>>
>> So far I could not find out how to query this from the firmware.
>> The IT8688 chip can report the state of each sensor but that is not exposed 
>> by
>> the firmware.
>> But even the state information from the IT8688 is not accurate as is.
>> One of the sensors that is reported as being active (directly via it87) on my
>> machine always reports -55°C (yes, negative).
> 
> Ok.
> 
>>> Perhaps the WMI interface returns an error when you query an out-of-range
>>> temperature channel?
>>
>> Also "0" as mentioned above.
> 
> Hmm, so maybe this can be used to limit the amount of reported temperature
> sensors, IOW if sensors 5 and 6 report 0, only register 4 sensors ?
> 
>>
>>> One option here might be to add a DMI matching table and only load on
>>> systems on that table for now. That table could then perhaps also provide
>>> labels for each of the temperature channels, which is something which
>>> would be nice to have regardless of my worries about how well this driver
>>> will work on motherboards on which it has not been tested.
>>
>> I am collecting reports for working motherboards at
>> https://github.com/t-8ch/linux-gigabyte-wmi-driver/issues/1 .
> 
> Good, you should probably ask reporters there to provide the output of:
> 
> grep . /sys/class/dmi/id/* 2> /dev/null
> 
> Ran as a normal user (so that the serial-numbers will be skipped) so that
> you will have DMI strings to match on if you decide to go that route.
> 
>>
>>> You could combine this DMI matching table with a "force" module option to
>>> continue with probing on boards which are not on the table to allow users
>>> to test and report their results to you.
>>>
>>> And hopefully after a while, when we're confident that the code works
>>> well on most gigabyte boards we can drop the DMI table, or at least
>>> only use it for the channel labels.
>>
>> That sounds good.
>>
>>> Please don't take this the wrong way; I 

Re: [PATCH 1/1] usb: typec: tcpm: remove unused static variable 'tcpm_altmode_ops'

[Guenter Roeck]

On 4/8/21 1:28 AM, Heikki Krogerus wrote:
> On Wed, Apr 07, 2021 at 05:15:40PM +0800, Zhen Lei wrote:
>> Fixes the following W=1 kernel build warning:
>>
>> drivers/usb/typec/tcpm/tcpm.c:2107:39: warning: ‘tcpm_altmode_ops’ defined 
>> but not used [-Wunused-const-variable=]
>>
>> The reference to the variable 'tcpm_altmode_ops' is deleted by the
>> commit a079973f462a ("usb: typec: tcpm: Remove tcpc_config configuration
>> mechanism").
>>
>> By the way, the static functions referenced only by the variable
>> 'tcpm_altmode_ops' are deleted accordingly.
>>
>> Reported-by: Hulk Robot 
>> Signed-off-by: Zhen Lei 
> 
> Oh, I thought this was already fixed. Should this go into the stable
> trees as well?
> 

I thought there was some code supposed to be coming which would make use of it,
but my memory may defeat me. Either case, it is getting old. It it is ever 
needed,
it can be reintroduced.

Reviewed-by: Guenter Roeck 

Guenter

> Acked-by: Heikki Krogerus 
> 
>> ---
>>  drivers/usb/typec/tcpm/tcpm.c | 60 
>> ---
>>  1 file changed, 60 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>> index ce7af398c7c1c1f..2f89bae29c0c297 100644
>> --- a/drivers/usb/typec/tcpm/tcpm.c
>> +++ b/drivers/usb/typec/tcpm/tcpm.c
>> @@ -1365,14 +1365,6 @@ static void tcpm_queue_vdm(struct tcpm_port *port, 
>> const u32 header,
>>  mod_vdm_delayed_work(port, 0);
>>  }
>>  
>> -static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 
>> header,
>> -const u32 *data, int cnt)
>> -{
>> -mutex_lock(>lock);
>> -tcpm_queue_vdm(port, header, data, cnt);
>> -mutex_unlock(>lock);
>> -}
>> -
>>  static void svdm_consume_identity(struct tcpm_port *port, const u32 *p, int 
>> cnt)
>>  {
>>  u32 vdo = p[VDO_INDEX_IDH];
>> @@ -1705,8 +1697,6 @@ static void tcpm_handle_vdm_request(struct tcpm_port 
>> *port,
>>   *
>>   * And we also have this ordering:
>>   * 1. alt-mode driver takes the alt-mode's lock
>> - * 2. alt-mode driver calls tcpm_altmode_enter which takes the
>> - *tcpm port lock
>>   *
>>   * Dropping our lock here avoids this.
>>   */
>> @@ -2060,56 +2050,6 @@ static int tcpm_validate_caps(struct tcpm_port *port, 
>> const u32 *pdo,
>>  return 0;
>>  }
>>  
>> -static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
>> -{
>> -struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
>> -int svdm_version;
>> -u32 header;
>> -
>> -svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
>> -if (svdm_version < 0)
>> -return svdm_version;
>> -
>> -header = VDO(altmode->svid, vdo ? 2 : 1, svdm_version, CMD_ENTER_MODE);
>> -header |= VDO_OPOS(altmode->mode);
>> -
>> -tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
>> -return 0;
>> -}
>> -
>> -static int tcpm_altmode_exit(struct typec_altmode *altmode)
>> -{
>> -struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
>> -int svdm_version;
>> -u32 header;
>> -
>> -svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
>> -if (svdm_version < 0)
>> -return svdm_version;
>> -
>> -header = VDO(altmode->svid, 1, svdm_version, CMD_EXIT_MODE);
>> -header |= VDO_OPOS(altmode->mode);
>> -
>> -tcpm_queue_vdm_unlocked(port, header, NULL, 0);
>> -return 0;
>> -}
>> -
>> -static int tcpm_altmode_vdm(struct typec_altmode *altmode,
>> -u32 header, const u32 *data, int count)
>> -{
>> -struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
>> -
>> -tcpm_queue_vdm_unlocked(port, header, data, count - 1);
>> -
>> -return 0;
>> -}
>> -
>> -static const struct typec_altmode_ops tcpm_altmode_ops = {
>> -.enter = tcpm_altmode_enter,
>> -.exit = tcpm_altmode_exit,
>> -.vdm = tcpm_altmode_vdm,
>> -};
>> -
>>  /*
>>   * PD (data, control) command handling functions
>>   */
>> -- 
>> 1.8.3
>>
> 

Re: [PATCH v2] hwmon: Add driver for fsp-3y PSUs and PDUs

[Guenter Roeck]

On 4/7/21 7:34 PM, Václav Kubernát wrote:
> This patch adds support for these devices:
> - YH-5151E - the PDU
> - YM-2151E - the PSU
> 
> The device datasheet says that the devices support PMBus 1.2, but in my
> testing, a lot of the commands aren't supported and if they are, they
> sometimes behave strangely or inconsistently. For example, writes to the
> PAGE command requires using PEC, otherwise the write won't work and the
> page won't switch, even though, the standard says that PEC is opiotnal.
> On the other hand, writes the SMBALERT don't require PEC. Because of
> this, the driver is mostly reverse engineered with the help of a tool
> called pmbus_peek written by David Brownell (and later adopted by my
> colleague Jan Kundrát).
> 
> The device also has some sort of a timing issue when switching pages,
> which is explained further in the code.
> 
> Because of this, the driver support is limited. It exposes only the
> values, that have been tested to work correctly.
> 
> Signed-off-by: Václav Kubernát 
> ---
>  Documentation/hwmon/fsp-3y.rst |  26 
>  drivers/hwmon/pmbus/Kconfig|  10 ++
>  drivers/hwmon/pmbus/Makefile   |   1 +
>  drivers/hwmon/pmbus/fsp-3y.c   | 217 +
>  4 files changed, 254 insertions(+)
>  create mode 100644 Documentation/hwmon/fsp-3y.rst
>  create mode 100644 drivers/hwmon/pmbus/fsp-3y.c
> 
> diff --git a/Documentation/hwmon/fsp-3y.rst b/Documentation/hwmon/fsp-3y.rst
> new file mode 100644
> index ..68a547021846
> --- /dev/null
> +++ b/Documentation/hwmon/fsp-3y.rst
> @@ -0,0 +1,26 @@
> +Kernel driver fsp3y
> +==
> +Supported devices:
> +  * 3Y POWER YH-5151E
> +  * 3Y POWER YM-2151E
> +
> +Author: Václav Kubernát 
> +
> +Description
> +---
> +This driver implements limited support for two 3Y POWER devices.
> +
> +Sysfs entries
> +-
> +in1_inputinput voltage
> +in2_input12V output voltage
> +in3_input5V output voltage
> +curr1_input  input current
> +curr2_input  12V output current
> +curr3_input  5V output current
> +fan1_input   fan rpm
> +temp1_input  temperature 1
> +temp2_input  temperature 2
> +temp3_input  temperature 3
> +power1_input input power
> +power2_input output power
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 03606d4298a4..9d12d446396c 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -56,6 +56,16 @@ config SENSORS_BEL_PFE
> This driver can also be built as a module. If so, the module will
> be called bel-pfe.
>  
> +config SENSORS_FSP_3Y
> + tristate "FSP/3Y-Power power supplies"
> + help
> +   If you say yes here you get hardware monitoring support for
> +   FSP/3Y-Power hot-swap power supplies.
> +   Supported models: YH-5151E, YM-2151E
> +
> +   This driver can also be built as a module. If so, the module will
> +   be called fsp-3y.
> +
>  config SENSORS_IBM_CFFPS
>   tristate "IBM Common Form Factor Power Supply"
>   depends on LEDS_CLASS
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 6a4ba0fdc1db..bfe218ad898f 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS)   += pmbus.o
>  obj-$(CONFIG_SENSORS_ADM1266)+= adm1266.o
>  obj-$(CONFIG_SENSORS_ADM1275)+= adm1275.o
>  obj-$(CONFIG_SENSORS_BEL_PFE)+= bel-pfe.o
> +obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o
>  obj-$(CONFIG_SENSORS_IBM_CFFPS)  += ibm-cffps.o
>  obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
>  obj-$(CONFIG_SENSORS_IR35221)+= ir35221.o
> diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c
> new file mode 100644
> index ..2c165e034fa8
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/fsp-3y.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Hardware monitoring driver for FSP 3Y-Power PSUs
> + *
> + * Copyright (c) 2021 Václav Kubernát, CESNET
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "pmbus.h"
> +
> +#define YM2151_PAGE_12V_LOG  0x00
> +#define YM2151_PAGE_12V_REAL 0x00
> +#define YM2151_PAGE_5VSB_LOG 0x01
> +#define YM2151_PAGE_5VSB_REAL0x20
> +#define YH5151E_PAGE_12V_LOG 0x00
> +#define YH5151E_PAGE_12V_REAL0x00
> +#define YH5151E_PAGE_5V_LOG  0x01
> +#define YH5151E_PAGE_5V_REAL 0x10
> +#define YH5151E_PAGE_3V3_LOG 0x02
> +#define YH5151E_PAGE_3V3_REAL0x11
> +
> +enum chips {
> + ym2151e,
> + yh5151e
> +};
> +
> +struct fsp3y_data {
> + struct pmbus_driver_info info;
> + enum chips chip;
> + int page;
> +};
> +
> +#define to_fsp3y_data(x) container_of(x, struct fsp3y_data, info)
> +
> +static int page_log_to_page_real(int page_log, enum chips chip)
> +{
> + switch (chip) {
> + case 

Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/7/21 1:59 PM, Frank Rowand wrote:
> Hi Guenter,
> 
> On 4/7/21 3:51 PM, frowand.l...@gmail.com wrote:
>> From: Frank Rowand 
>>
>> The Devicetree standard specifies an 8 byte alignment of the FDT.
>> Code in libfdt expects this alignment for an FDT image in memory.
>> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
>> with kmalloc(), align pointer, memcpy() to get proper alignment.
>>
>> The 4 byte alignment exposed a related bug which triggered a crash
>> on openrisc with:
>> commit 79edff12060f ("scripts/dtc: Update to upstream version 
>> v1.6.0-51-g183df9e9c2b9")
>> as reported in:
>> https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/
> 
> Can you please test this patch?
> 

Sure, will do, after you fixed the problem pointed out by Rob.

Sorry, I should have mentioned it - that problem was the reason
why I didn't propose a fix myself.

Guenter

Re: [PATCH v4 2/2] usb: dwc3: Add driver for Xilinx platforms

[Guenter Roeck]

On Wed, Mar 17, 2021 at 12:22:29PM +0530, Manish Narani wrote:
> Add a new driver for supporting Xilinx platforms. This driver is used
> for some sequence of operations required for Xilinx USB controllers.
> This driver is also used to choose between PIPE clock coming from SerDes
> and the Suspend Clock. Before the controller is out of reset, the clock
> selection should be changed to PIPE clock in order to make the USB
> controller work. There is a register added in Xilinx USB controller
> register space for the same.
> 
> Signed-off-by: Manish Narani 

Trying this driver with qemu (v6.0.0-rc2) results in:

[   15.184242] dwc3-xilinx ff9d.usb: error -ENODEV: failed to assert Reset
[   15.185754] Unable to handle kernel paging request at virtual address 
006b6b6b6b6b6b9b
[   15.185994] Mem abort info:
[   15.186065]   ESR = 0x9604
[   15.186317]   EC = 0x25: DABT (current EL), IL = 32 bits
[   15.186414]   SET = 0, FnV = 0
[   15.186498]   EA = 0, S1PTW = 0
[   15.186579] Data abort info:
[   15.18]   ISV = 0, ISS = 0x0004
[   15.186756]   CM = 0, WnR = 0
[   15.186887] [006b6b6b6b6b6b9b] address between user and kernel address ranges
[   15.187436] Internal error: Oops: 9604 [#1] PREEMPT SMP
[   15.18] Modules linked in:
[   15.188060] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.12.0-rc6-next-20210406-6-g05407f068fc9-dirty #1
[   15.188265] Hardware name: Xilinx Versal Virtual development board (DT)
[   15.188495] pstate: 8005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[   15.188614] pc : __clk_put+0x24/0x138
[   15.188716] lr : __clk_put+0x24/0x138
[   15.188791] sp : 80001326bac0
[   15.188853] x29: 80001326bac0 x28: 0644ed00
[   15.188982] x27: 0421ecd0 x26: 0421e810
[   15.189076] x25: 0644f100 x24: 
[   15.189170] x23: 8000126a2570 x22: 0005
[   15.189271] x21: 0644ed00 x20: 06449970
[   15.189367] x19: 6b6b6b6b6b6b6b6b x18: 0010
[   15.189456] x17: 0001 x16: 
[   15.189546] x15: 03af0490 x14: 01b7
[   15.189642] x13: 03af0490 x12: ffea
[   15.189729] x11: 8000123b6460 x10: 0080
[   15.189815] x9 : 676993c6 x8 : 676993c6
[   15.189941] x7 : 7d152ab3 x6 : 800012768480
[   15.190047] x5 :  x4 : 7f97631e
[   15.190139] x3 : d5bdf2c2 x2 : 000b
[   15.190233] x1 : 03af0040 x0 : 0001
[   15.190432] Call trace:
[   15.190506]  __clk_put+0x24/0x138
[   15.190588]  clk_put+0x10/0x20
[   15.190653]  clk_bulk_put+0x3c/0x60
[   15.190724]  devm_clk_bulk_release+0x1c/0x28
[   15.190806]  release_nodes+0x1c0/0x2b0
[   15.190887]  devres_release_all+0x38/0x60
[   15.190963]  really_probe+0x1e4/0x3a8
[   15.191042]  driver_probe_device+0x64/0xc8
...

because of ...

> +
> + ret = devm_clk_bulk_get_all(priv_data->dev, _data->clks);
> + if (ret < 0)
> + return ret;
> +
...
> +
> +err_clk_put:
> + clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
> + clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);

clk_bulk_put_all() is not necessary because of devm_clk_bulk_get_all(),
and results in a double free.

> +static int dwc3_xlnx_remove(struct platform_device *pdev)
> +{
> + struct dwc3_xlnx*priv_data = platform_get_drvdata(pdev);
> + struct device   *dev = >dev;
> +
> + of_platform_depopulate(dev);
> +
> + clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
> + clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);

Same here. This will likely crash the driver on unload.

Guenter

Re: [PATCH] MIPS: select ARCH_KEEP_MEMBLOCK unconditionally

[Guenter Roeck]

On Wed, Apr 07, 2021 at 10:35:43AM -0700, Nick Desaulniers wrote:
> While removing allnoconfig_y from Kconfig, ARCH=mips allnoconfig builds
> started failing with the error:
> 
> WARNING: modpost: vmlinux.o(.text+0x9c70): Section mismatch in reference
> from the function reserve_exception_space() to the function
> .meminit.text:memblock_reserve()
> The function reserve_exception_space() references the function __meminit
> memblock_reserve().
> This is often because reserve_exception_space lacks a __meminit
> annotation or the annotation of memblock_reserve is wrong.
> ERROR: modpost: Section mismatches detected.
> Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.
> 
> allnoconfig disables DEBUG_KERNEL and thus ARCH_KEEP_MEMBLOCK, which
> changes __init_memblock to be equivalent to __meminit triggering the
> above error.
> 
> Link: 
> https://lore.kernel.org/linux-kbuild/20210313194836.372585-11-masahi...@kernel.org/
> Fixes: commit a8c0f1c634507 ("MIPS: Select ARCH_KEEP_MEMBLOCK if

s/commit //

> DEBUG_KERNEL to enable sysfs memblock debug")
> Cc: Masahiro Yamada 
> Reported-by: Guenter Roeck 
> Signed-off-by: Nick Desaulniers 

Tested-by: Guenter Roeck 

Guenter

> ---
>  arch/mips/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index e9893cd34992..702648f60e41 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -12,7 +12,7 @@ config MIPS
>   select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>   select ARCH_HAS_UBSAN_SANITIZE_ALL
>   select ARCH_HAS_GCOV_PROFILE_ALL
> - select ARCH_KEEP_MEMBLOCK if DEBUG_KERNEL
> + select ARCH_KEEP_MEMBLOCK
>   select ARCH_SUPPORTS_UPROBES
>   select ARCH_USE_BUILTIN_BSWAP
>   select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
> -- 
> 2.31.1.295.g9ea45b61b8-goog
> 

Re: [PATCH v2 1/1] serial: sh-sci: Respect deferred probe when getting IRQ

[Guenter Roeck]

On 4/7/21 3:17 AM, Andy Shevchenko wrote:
> With platform_get_irq() and its optional variant it's possible to get
> a deferred probe error code. Since the commit ed7027fdf4ec ("driver core:
> platform: Make platform_get_irq_optional() optional") the error code
> can be distinguished from no IRQ case. With this, rewrite IRQ resource
> handling in sh-sci driver to follow above and allow to respect deferred
> probe.
> 
> Fixes: ed7027fdf4ec ("driver core: platform: Make platform_get_irq_optional() 
> optional")
> Reported-by: Guenter Roeck 
> Signed-off-by: Andy Shevchenko 

This patch alone causes a hard hang early during boot. It works if applied
together with ed7027fdf4ec. Ultimately that means that ed7027fdf4ec introduces
a functional change, and will need to be applied very carefully. A cursory
glance through callers of platform_get_irq_optional() shows that many
do not handle this correctly: various drivers handle a return value of 0
as valid interrupt, and others treat errors other than -ENXIO as fatal.

Also, each patch on its own causes failures on sh, which is problematic
when applying them even as series. See below for an idea how to
address that.

> ---
> v2: fixed a typo: i -> 0
>  drivers/tty/serial/sh-sci.c | 18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index ad2c189e8fc8..574f68ba50ff 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2899,13 +2899,6 @@ static int sci_init_single(struct platform_device *dev,
>   port->mapbase = res->start;
>   sci_port->reg_size = resource_size(res);
>  
> - for (i = 0; i < ARRAY_SIZE(sci_port->irqs); ++i) {
> - if (i)
> - sci_port->irqs[i] = platform_get_irq_optional(dev, i);
> - else
> - sci_port->irqs[i] = platform_get_irq(dev, i);
> - }
> -
>   /* The SCI generates several interrupts. They can be muxed together or
>* connected to different interrupt lines. In the muxed case only one
>* interrupt resource is specified as there is only one interrupt ID.
> @@ -2913,12 +2906,17 @@ static int sci_init_single(struct platform_device 
> *dev,
>* from the SCI, however those signals might have their own individual
>* interrupt ID numbers, or muxed together with another interrupt.
>*/
> + sci_port->irqs[0] = platform_get_irq(dev, 0);
>   if (sci_port->irqs[0] < 0)
> - return -ENXIO;
> + return sci_port->irqs[0];
>  
> - if (sci_port->irqs[1] < 0)
> - for (i = 1; i < ARRAY_SIZE(sci_port->irqs); i++)
> + for (i = 1; i < ARRAY_SIZE(sci_port->irqs); ++i) {
> + sci_port->irqs[i] = platform_get_irq_optional(dev, i);
> + if (sci_port->irqs[i] < 0)
> + return sci_port->irqs[i];
> + if (sci_port->irqs[i] == 0)
>   sci_port->irqs[i] = sci_port->irqs[0];

Since sh never gets -EPROBE_DEFER, the following code can be applied
on its own and does not depend on ed7027fdf4ec.

sci_port->irqs[i] = platform_get_irq_optional(dev, i);
if (sci_port->irqs[i] <= 0)
sci_port->irqs[i] = sci_port->irqs[0];

With this change, sh images boot in qemu both with and without ed7027fdf4ec.

Thanks,
Guenter

> + }
>  
>   sci_port->params = sci_probe_regmap(p);
>   if (unlikely(sci_port->params == NULL))
> 

Re: [PATCH v3 3/4] watchdog: simatic-ipc-wdt: add new driver for Siemens Industrial PCs

[Guenter Roeck]

On 4/7/21 1:53 AM, Andy Shevchenko wrote:
> On Tue, Apr 6, 2021 at 5:56 PM Henning Schild
>  wrote:
>>
>> Am Thu, 1 Apr 2021 18:15:41 +0200
>> schrieb "Enrico Weigelt, metux IT consult" :
>>
>>> On 29.03.21 19:49, Henning Schild wrote:
>>>
>>> Hi,
>>>
 This driver adds initial support for several devices from Siemens.
 It is based on a platform driver introduced in an earlier commit.
>>>
>>> Where does the wdt actually come from ?
>>>
>>> Is it in the SoC ? (which SoC exactly). SoC-builtin wdt is a pretty
>>> usual case.
>>>
>>> Or some external chip ?
>>
>> I guess external chip, but again we are talking about multiple
>> machines. And the manuals i read so far do not go into that sort of
>> detail. In fact on some of the machines you will have two watchdogs,
>> one from the SoC and that "special" one.
>> That has several reasons, probably not too important here. The HW guys
>> are adding another wd not just for fun, and it would be nice to have a
>> driver.
>>
>>> The code smells a bit like two entirely different wdt's that just have
>>> some similarities. If that's the case, I'd rather split it into two
>>> separate drivers and let the parent driver (board file) instantiate
>>> the correct one.
>>
>> Yes, it is two. Just like for the LEDs. One version PIO-based another
>> version gpio/p2sb/mmio based.
> 
> I tend to agree with Enrico that this should be two separate drivers.
> 

Agreed.

Guenter

>> In fact the latter should very likely be based on that gpio pinctl,
>> whether it really needs to be a separate driver will have to be seen.
>> There are probably pros and cons for both options.
> 
> 

Re: [PATCH v2 07/19] dt-bindings: fix references for iio-bindings.txt

[Guenter Roeck]

On 4/7/21 1:20 AM, Mauro Carvalho Chehab wrote:
> The iio-bindings.txt was converted into two files and merged
> at the dt-schema git tree at:
> 
>   https://github.com/devicetree-org/dt-schema
> 
> Yet, some documents still refer to the old file. Fix their
> references, in order to point to the right URL.
> 
> Fixes: dba91f82d580 ("dt-bindings:iio:iio-binding.txt Drop file as content 
> now in dt-schema")
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt   | 2 +-

Acked-by: Guenter Roeck 

>  Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml   | 5 +++--
>  Documentation/devicetree/bindings/input/adc-joystick.yaml| 4 +++-
>  .../bindings/input/touchscreen/resistive-adc-touch.txt   | 5 -
>  Documentation/devicetree/bindings/mfd/ab8500.txt | 4 +++-
>  .../devicetree/bindings/power/supply/da9150-charger.txt  | 2 +-
>  6 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt 
> b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
> index 37f18d684f6a..4c5c3712970e 100644
> --- a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
> +++ b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt
> @@ -32,7 +32,7 @@ Optional node properties:
>  - "#thermal-sensor-cells" Used to expose itself to thermal fw.
>  
>  Read more about iio bindings at
> - Documentation/devicetree/bindings/iio/iio-bindings.txt
> + https://github.com/devicetree-org/dt-schema/blob/master/schemas/iio/
>  
>  Example:
>   ncp15wb473@0 {
> diff --git a/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml 
> b/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml
> index 9f414dbdae86..433a3fb55a2e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml
> @@ -14,8 +14,9 @@ description: >
>Industrial I/O subsystem bindings for ADC controller found in
>Ingenic JZ47xx SoCs.
>  
> -  ADC clients must use the format described in iio-bindings.txt, giving
> -  a phandle and IIO specifier pair ("io-channels") to the ADC controller.
> +  ADC clients must use the format described in
> +  
> https://github.com/devicetree-org/dt-schema/blob/master/schemas/iio/iio-consumer.yaml,
> +  giving a phandle and IIO specifier pair ("io-channels") to the ADC 
> controller.
>  
>  properties:
>compatible:
> diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml 
> b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> index 054406bbd22b..721878d5b7af 100644
> --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> @@ -24,7 +24,9 @@ properties:
>  description: >
>List of phandle and IIO specifier pairs.
>Each pair defines one ADC channel to which a joystick axis is 
> connected.
> -  See Documentation/devicetree/bindings/iio/iio-bindings.txt for details.
> +  See
> +  
> https://github.com/devicetree-org/dt-schema/blob/master/schemas/iio/iio-consumer.yaml
> +  for details.
>  
>'#address-cells':
>  const: 1
> diff --git 
> a/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt 
> b/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
> index fee0da12474e..af5223bb5bdd 100644
> --- 
> a/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
> +++ 
> b/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
> @@ -5,7 +5,10 @@ Required properties:
>   - compatible: must be "resistive-adc-touch"
>  The device must be connected to an ADC device that provides channels for
>  position measurement and optional pressure.
> -Refer to Documentation/devicetree/bindings/iio/iio-bindings.txt for details
> +Refer to
> +https://github.com/devicetree-org/dt-schema/blob/master/schemas/iio/iio-consumer.yaml
> +for details
> +
>   - iio-channels: must have at least two channels connected to an ADC device.
>  These should correspond to the channels exposed by the ADC device and should
>  have the right index as the ADC device registers them. These channels
> diff --git a/Documentation/devicetree/bindings/mfd/ab8500.txt 
> b/Documentation/devicetree/bindings/mfd/ab8500.txt
> index d2a6e835c257..937b3e5505e0 100644
> --- a/Documentation/devicetree/bindings/mfd/ab8500.txt
> +++ b/Documentation/devicetree/bindings/mfd/ab8500.txt
> @@ -72,7 +72,9 @@ Required child device properties:
>   

Re: [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver

[Guenter Roeck]

On 4/7/21 12:41 AM, Quan Nguyen wrote:
[ ... ]
>>
>> But then why don't you just use reg_ext to store SOC_VR_HOT_THRESHOLD_REG
>> or MEM_HOT_THRESHOLD_REG ? It is already available, after all, and with it
>> the code could be simplified to
>>
>>     ret = regmap_read(hwmon->regmap, temperature[channel].reg_ext, 
>> );
>>     if (ret)
>>     return ret;
>>
> Thank you for the comment.
> 
> Will change code follow this suggestion, will include in next version
> 
>> I don't have a datasheet, but I do wonder what is in bit 9..15. Any idea ?
>> Main question is if there is a sign bit, as theoretic as it may be.
>>
> The original intention was to use this as 9-bit 2-complement value follow 
> LM75, but the fact is that the operation temperature is 0-125 C degree, so we 
> simply use it as-is.
> 

The operational temperature is not the question here. The question is if the
chip _reports_ a sign. If it does, it should be handled, even if it is outside
the operational range. The reported range is relevant here, not the operational
range. After all, the chip won't really blow apart at -1 degrees C.

Thanks,
Guenter

Re: [PATCH v2] hwmon: (sbtsi) Don't read sensor more than once if it doesn't respond

[Guenter Roeck]

On 4/6/21 3:25 PM, Konstantin Aladyshev wrote:
> What I'm trying to say, shouldn't the call
> "i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG)" be
> surrounded with the "mutex_lock/mutex_unlock" like it is done for the
> others "i2c_smbus_read_byte_data" calls?
> Something like:
> ```
> mutex_lock(>lock);
> err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
> if (err < 0)
> return err;
> mutex_unlock(>lock);
> ```
> Because it is not surrounded with the mutex lock/unlock in the original 
> driver.
> 

Why would that be necessary ? My understanding is that the returned value
is read-only and won't change. FWIW, I don't even understand why it is
read more than once to start with.

On a side note, the above code would leave the mutex locked on error.

Guenter

> Best regards,
> Konstantin Aladyshev
> 
> 
> On Wed, Apr 7, 2021 at 12:34 AM Guenter Roeck  wrote:
>>
>> On 4/6/21 2:09 PM, Konstantin Aladyshev wrote:
>>> Thanks for the answer!
>>> Sorry for the confusion, by the "CPU is off" I meant "CPU is present,
>>> but currently it is in the powered off state".
>>> Therefore it is not possible to put these checks only in a probe
>>> function. And I don't know either if it is a good idea to cache
>>> config/min/max values.
>>>
>>> I use this driver on an OpenBMC system, which uses other software
>>> rather than lm-sensors utility. I guess that is why my priorities are
>>> shifted.
>>>
>>> By the way, I've noticed that the mutex check is absent in a
>>> SBTSI_REG_CONFIG read call both in the original driver version and in
>>> my patch, is this an error?
>>>
>>
>> What do you mean with "mutex check" ?
>>
>> Thanks,
>> Guenter
>>
>>
>>> Best regards,
>>> Konstantin Aladyshev
>>>
>>>
>>> On Tue, Apr 6, 2021 at 11:09 PM Guenter Roeck  wrote:
>>>>
>>>> On 4/6/21 12:20 PM, Konstantin Aladyshev wrote:
>>>>> Thanks for the comment.
>>>>> Yes, you are correct, this patch adds an extra 'i2c_smbus_read_byte_data' 
>>>>> call for the temp_max/temp_min reads.
>>>>> I guess I did that intentionally because I just wanted to keep the 
>>>>> restructured code concise. After all I thought, 'temp_input' generally is 
>>>>> read more often than 'temp_max/temp_min'.
>>>>> As I understand now, it seems like it is not acceptable. Therefore could 
>>>>> you point me in the right direction about what I should do?
>>>>> Should I just stick with the original driver version and simply add two 
>>>>> more i2c call checks for the first operations for min/max?
>>>>>
>>>>
>>>> Correct, it is not acceptable. A normal use case for hwmon devices is to 
>>>> use the "sensors"
>>>> command which _will_ read all attributes. i2c reads are expensive, and 
>>>> unnecessary read
>>>> operations should be avoided.
>>>>
>>>> There are several ways to solve the problem. Checking return values after 
>>>> each
>>>> read is the simple option. There are other possibilities, such as reading 
>>>> the limits
>>>> and the read order only once during probe, but I don't know enough about 
>>>> the
>>>> hardware to suggest a more sophisticated solution. For example, I don't 
>>>> know
>>>> what "CPU is off" means. Offline ? Not present ? If it means "not present",
>>>> or if the status is permanent, the condition should be handled in the 
>>>> is_visible
>>>> function (or the driver should not be instantiated in the first place).
>>>> Otherwise, the code should possibly return -ENODATA instead of -ETIMEDOUT
>>>> on error. But, again, I can not really suggest a better solution since
>>>> I don't know enough (nothing, actually) about the hardware (and the public
>>>> part of the SBTSI specification doesn't say anything about expected 
>>>> behavior
>>>> for offline CPUs or CPU cores).
>>>>
>>>> What I did find, though, is that the driver does not implement temperature
>>>> offset support, and it that doesn't support reporting alerts. I'd have 
>>>> assumed
>>>> this to be more important than optimizing error handling, but that is just
>>>> my personal opinion.
>>>>
>>>>

Re: [PATCH v2] h8300: rearrange headers inclusion order in asm/bitops

[Guenter Roeck]

On Tue, Apr 06, 2021 at 11:36:25AM -0700, Yury Norov wrote:
> The commit a5145bdad3ff ("arch: rearrange headers inclusion order in
> asm/bitops for m68k and sh") on next-20210401 fixed header ordering issue.
> h8300 has similar problem, which was overlooked by me.
> 
> h8300 includes bitmap/{find,le}.h prior to ffs/fls headers. New fast-path
> implementation in find.h requires ffs/fls. Reordering the headers inclusion
> sequence helps to prevent compile-time implicit function declaration error.
> 
> v2: change wording in the comment.
> 
> Reported-by: kernel test robot 
> Reported-by: Guenter Roeck 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Yury Norov 

Tested-by: Guenter Roeck 

Guenter

> ---
>  arch/h8300/include/asm/bitops.h | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/h8300/include/asm/bitops.h b/arch/h8300/include/asm/bitops.h
> index 7aa16c732aa9..c867a80cab5b 100644
> --- a/arch/h8300/include/asm/bitops.h
> +++ b/arch/h8300/include/asm/bitops.h
> @@ -9,6 +9,10 @@
>  
>  #include 
>  
> +#include 
> +#include 
> +#include 
> +
>  #ifdef __KERNEL__
>  
>  #ifndef _LINUX_BITOPS_H
> @@ -173,8 +177,4 @@ static inline unsigned long __ffs(unsigned long word)
>  
>  #endif /* __KERNEL__ */
>  
> -#include 
> -#include 
> -#include 
> -
>  #endif /* _H8300_BITOPS_H */
> -- 
> 2.25.1
> 

Re: [PATCH v2] hwmon: (sbtsi) Don't read sensor more than once if it doesn't respond

[Guenter Roeck]

On 4/6/21 2:09 PM, Konstantin Aladyshev wrote:
> Thanks for the answer!
> Sorry for the confusion, by the "CPU is off" I meant "CPU is present,
> but currently it is in the powered off state".
> Therefore it is not possible to put these checks only in a probe
> function. And I don't know either if it is a good idea to cache
> config/min/max values.
> 
> I use this driver on an OpenBMC system, which uses other software
> rather than lm-sensors utility. I guess that is why my priorities are
> shifted.
> 
> By the way, I've noticed that the mutex check is absent in a
> SBTSI_REG_CONFIG read call both in the original driver version and in
> my patch, is this an error?
> 

What do you mean with "mutex check" ?

Thanks,
Guenter


> Best regards,
> Konstantin Aladyshev
> 
> 
> On Tue, Apr 6, 2021 at 11:09 PM Guenter Roeck  wrote:
>>
>> On 4/6/21 12:20 PM, Konstantin Aladyshev wrote:
>>> Thanks for the comment.
>>> Yes, you are correct, this patch adds an extra 'i2c_smbus_read_byte_data' 
>>> call for the temp_max/temp_min reads.
>>> I guess I did that intentionally because I just wanted to keep the 
>>> restructured code concise. After all I thought, 'temp_input' generally is 
>>> read more often than 'temp_max/temp_min'.
>>> As I understand now, it seems like it is not acceptable. Therefore could 
>>> you point me in the right direction about what I should do?
>>> Should I just stick with the original driver version and simply add two 
>>> more i2c call checks for the first operations for min/max?
>>>
>>
>> Correct, it is not acceptable. A normal use case for hwmon devices is to use 
>> the "sensors"
>> command which _will_ read all attributes. i2c reads are expensive, and 
>> unnecessary read
>> operations should be avoided.
>>
>> There are several ways to solve the problem. Checking return values after 
>> each
>> read is the simple option. There are other possibilities, such as reading 
>> the limits
>> and the read order only once during probe, but I don't know enough about the
>> hardware to suggest a more sophisticated solution. For example, I don't know
>> what "CPU is off" means. Offline ? Not present ? If it means "not present",
>> or if the status is permanent, the condition should be handled in the 
>> is_visible
>> function (or the driver should not be instantiated in the first place).
>> Otherwise, the code should possibly return -ENODATA instead of -ETIMEDOUT
>> on error. But, again, I can not really suggest a better solution since
>> I don't know enough (nothing, actually) about the hardware (and the public
>> part of the SBTSI specification doesn't say anything about expected behavior
>> for offline CPUs or CPU cores).
>>
>> What I did find, though, is that the driver does not implement temperature
>> offset support, and it that doesn't support reporting alerts. I'd have 
>> assumed
>> this to be more important than optimizing error handling, but that is just
>> my personal opinion.
>>
>> Thanks,
>> Guenter
>>
>>> Best regards,
>>> Konstantin Aladyshev
>>>
>>>
>>> On Tue, Apr 6, 2021 at 9:42 PM Guenter Roeck >> <mailto:li...@roeck-us.net>> wrote:
>>>
>>> On 4/6/21 11:16 AM, Konstantin Aladyshev wrote:
>>> > SBTSI sensors don't work when the CPU is off.
>>> > In this case every 'i2c_smbus_read_byte_data' function would fail
>>> > by a timeout.
>>> > Currently temp1_max/temp1_min file reads can cause two such timeouts
>>> > for every read.
>>> > Restructure code so there will be no more than one timeout for every
>>> > read operation.
>>> >
>>> > Signed-off-by: Konstantin Aladyshev >> <mailto:aladyshe...@gmail.com>>
>>> > ---
>>> > Changes in v2:
>>> >   - Fix typo in a commit message
>>> >   - Don't swap temp_int/temp_dec checks at the end of the 
>>> 'sbtsi_read' function
>>> >
>>>
>>> This doesn't explain the reason for the extra read operation for
>>> limits. Preventing a second read in error cases is not an argument
>>> for adding an extra read for non-error cases.
>>>
>>> Guenter
>>>
>>> >  drivers/hwmon/sbtsi_temp.c | 55 
>>> +++---
>>> >  1 file changed, 27 insertions(+),

Re: [PATCH v2] hwmon: (sbtsi) Don't read sensor more than once if it doesn't respond

[Guenter Roeck]

On 4/6/21 12:20 PM, Konstantin Aladyshev wrote:
> Thanks for the comment.
> Yes, you are correct, this patch adds an extra 'i2c_smbus_read_byte_data' 
> call for the temp_max/temp_min reads.
> I guess I did that intentionally because I just wanted to keep the 
> restructured code concise. After all I thought, 'temp_input' generally is 
> read more often than 'temp_max/temp_min'.
> As I understand now, it seems like it is not acceptable. Therefore could you 
> point me in the right direction about what I should do?
> Should I just stick with the original driver version and simply add two more 
> i2c call checks for the first operations for min/max?
> 

Correct, it is not acceptable. A normal use case for hwmon devices is to use 
the "sensors"
command which _will_ read all attributes. i2c reads are expensive, and 
unnecessary read
operations should be avoided.

There are several ways to solve the problem. Checking return values after each
read is the simple option. There are other possibilities, such as reading the 
limits
and the read order only once during probe, but I don't know enough about the
hardware to suggest a more sophisticated solution. For example, I don't know
what "CPU is off" means. Offline ? Not present ? If it means "not present",
or if the status is permanent, the condition should be handled in the is_visible
function (or the driver should not be instantiated in the first place).
Otherwise, the code should possibly return -ENODATA instead of -ETIMEDOUT
on error. But, again, I can not really suggest a better solution since
I don't know enough (nothing, actually) about the hardware (and the public
part of the SBTSI specification doesn't say anything about expected behavior
for offline CPUs or CPU cores).

What I did find, though, is that the driver does not implement temperature
offset support, and it that doesn't support reporting alerts. I'd have assumed
this to be more important than optimizing error handling, but that is just
my personal opinion.

Thanks,
Guenter

> Best regards,
> Konstantin Aladyshev
> 
> 
> On Tue, Apr 6, 2021 at 9:42 PM Guenter Roeck  <mailto:li...@roeck-us.net>> wrote:
> 
> On 4/6/21 11:16 AM, Konstantin Aladyshev wrote:
> > SBTSI sensors don't work when the CPU is off.
> > In this case every 'i2c_smbus_read_byte_data' function would fail
> > by a timeout.
> > Currently temp1_max/temp1_min file reads can cause two such timeouts
> > for every read.
> > Restructure code so there will be no more than one timeout for every
> > read operation.
> >
> > Signed-off-by: Konstantin Aladyshev  <mailto:aladyshe...@gmail.com>>
> > ---
> > Changes in v2:
> >   - Fix typo in a commit message
> >   - Don't swap temp_int/temp_dec checks at the end of the 'sbtsi_read' 
> function
> >
> 
> This doesn't explain the reason for the extra read operation for
> limits. Preventing a second read in error cases is not an argument
> for adding an extra read for non-error cases.
> 
> Guenter
> 
> >  drivers/hwmon/sbtsi_temp.c | 55 +++---
> >  1 file changed, 27 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
> > index e35357c48b8e..4ae48635bb31 100644
> > --- a/drivers/hwmon/sbtsi_temp.c
> > +++ b/drivers/hwmon/sbtsi_temp.c
> > @@ -74,48 +74,47 @@ static int sbtsi_read(struct device *dev, enum 
> hwmon_sensor_types type,
> >                     u32 attr, int channel, long *val)
> >  {
> >       struct sbtsi_data *data = dev_get_drvdata(dev);
> > +     u8 temp_int_reg, temp_dec_reg;
> >       s32 temp_int, temp_dec;
> >       int err;
> > 
> >       switch (attr) {
> >       case hwmon_temp_input:
> > -             /*
> > -              * ReadOrder bit specifies the reading order of integer 
> and
> > -              * decimal part of CPU temp for atomic reads. If bit == 0,
> > -              * reading integer part triggers latching of the decimal 
> part,
> > -              * so integer part should be read first. If bit == 1, read
> > -              * order should be reversed.
> > -              */
> > -             err = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_CONFIG);
> > -             if (err < 0)
> > -                     return err;
> > -
> > -             mutex_lock(>lock);
> > -             if (err & BIT(SBTSI_CONFIG_READ_ORDER_SHIFT)) {
> > -              

Re: [PATCH v1 1/1] driver core: platform: Make platform_get_irq_optional() optional

[Guenter Roeck]

On Wed, Mar 31, 2021 at 05:45:26PM +0300, Andy Shevchenko wrote:
> Currently the platform_get_irq_optional() returns an error code even
> if IRQ resource sumply has not been found. It prevents caller to be
> error code agnostic in their error handling.
> 
> Now:
>   ret = platform_get_irq_optional(...);
>   if (ret != -ENXIO)
>   return ret; // respect deferred probe
>   if (ret > 0)
>   ...we get an IRQ...
> 
> After proposed change:
>   ret = platform_get_irq_optional(...);
>   if (ret < 0)
>   return ret;
>   if (ret > 0)
>   ...we get an IRQ...
> 
> Reported-by: Matthias Schiffer 
> Signed-off-by: Andy Shevchenko 

This patch causes all my "sh" emulations to stall during boot with the
following repeated error message.

sh-sci sh-sci.1: Can't allocate rx full IRQ

Reverting this patch fixes the problem (and the message is gone).
Bisect log is attached.

Guenter

---
# bad: [9c54130cd25528028b2d38f6ada0c79e92210390] Add linux-next specific files 
for 20210406
# good: [e49d033bddf5b565044e2abe4241353959bc9120] Linux 5.12-rc6
git bisect start 'HEAD' 'v5.12-rc6'
# good: [3981dcd7199773fc8cfbbcc6173e3521b8e49035] Merge remote-tracking branch 
'crypto/master'
git bisect good 3981dcd7199773fc8cfbbcc6173e3521b8e49035
# good: [da18b6c82eba21e87d6ee76b7b0382eca123cc79] Merge remote-tracking branch 
'ftrace/for-next'
git bisect good da18b6c82eba21e87d6ee76b7b0382eca123cc79
# bad: [021e2b99a3cb523408609ca1792ab623ff16f334] Merge remote-tracking branch 
'staging/staging-next'
git bisect bad 021e2b99a3cb523408609ca1792ab623ff16f334
# bad: [685f903c62e3929370293bad184afa04b6fddebd] Merge remote-tracking branch 
'char-misc/char-misc-next'
git bisect bad 685f903c62e3929370293bad184afa04b6fddebd
# good: [67d49fe7e4d40cfe6919b434d6a4e837230af9d4] Merge remote-tracking branch 
'ipmi/for-next'
git bisect good 67d49fe7e4d40cfe6919b434d6a4e837230af9d4
# bad: [69e2ae87cfa94c77c3503715e9e0a68e6cc69f8d] Merge remote-tracking branch 
'usb/usb-next'
git bisect bad 69e2ae87cfa94c77c3503715e9e0a68e6cc69f8d
# good: [2665a13a3e9ef3d08b9ac4b48328ddfba9126987] usb: typec: Fix a typo
git bisect good 2665a13a3e9ef3d08b9ac4b48328ddfba9126987
# good: [967f6d162d9fa415cf140d3eef5576d566632292] dt-bindings: usb: mtk-xhci: 
remove redefinitions of usb3-lpm-capable
git bisect good 967f6d162d9fa415cf140d3eef5576d566632292
# good: [d225ef6fda7ce9ff7d28764bd1cceea2d0215e8b] base: dd: fix error return 
code of driver_sysfs_add()
git bisect good d225ef6fda7ce9ff7d28764bd1cceea2d0215e8b
# bad: [72a91f192da032b68519fafaecce03fd002d669a] driver core: add helper for 
deferred probe reason setting
git bisect bad 72a91f192da032b68519fafaecce03fd002d669a
# good: [1768289b44bae847612751d418fc5c5e680b5e5c] driver core: platform: 
Declare early_platform_cleanup() prototype
git bisect good 1768289b44bae847612751d418fc5c5e680b5e5c
# bad: [ed7027fdf4ec41ed6df6814956dc11860232a9d5] driver core: platform: Make 
platform_get_irq_optional() optional
git bisect bad ed7027fdf4ec41ed6df6814956dc11860232a9d5
# good: [318c3e00f13c2f6e11202a22cc302ea8c70552ea] driver core: Replace 
printf() specifier and drop unneeded casting
git bisect good 318c3e00f13c2f6e11202a22cc302ea8c70552ea
# first bad commit: [ed7027fdf4ec41ed6df6814956dc11860232a9d5] driver core: 
platform: Make platform_get_irq_optional() optional

Re: [PATCH v2] hwmon: (sbtsi) Don't read sensor more than once if it doesn't respond

[Guenter Roeck]

On 4/6/21 11:16 AM, Konstantin Aladyshev wrote:
> SBTSI sensors don't work when the CPU is off.
> In this case every 'i2c_smbus_read_byte_data' function would fail
> by a timeout.
> Currently temp1_max/temp1_min file reads can cause two such timeouts
> for every read.
> Restructure code so there will be no more than one timeout for every
> read operation.
> 
> Signed-off-by: Konstantin Aladyshev 
> ---
> Changes in v2:
>   - Fix typo in a commit message
>   - Don't swap temp_int/temp_dec checks at the end of the 'sbtsi_read' 
> function
> 

This doesn't explain the reason for the extra read operation for
limits. Preventing a second read in error cases is not an argument
for adding an extra read for non-error cases.

Guenter

>  drivers/hwmon/sbtsi_temp.c | 55 +++---
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
> index e35357c48b8e..4ae48635bb31 100644
> --- a/drivers/hwmon/sbtsi_temp.c
> +++ b/drivers/hwmon/sbtsi_temp.c
> @@ -74,48 +74,47 @@ static int sbtsi_read(struct device *dev, enum 
> hwmon_sensor_types type,
> u32 attr, int channel, long *val)
>  {
>   struct sbtsi_data *data = dev_get_drvdata(dev);
> + u8 temp_int_reg, temp_dec_reg;
>   s32 temp_int, temp_dec;
>   int err;
>  
>   switch (attr) {
>   case hwmon_temp_input:
> - /*
> -  * ReadOrder bit specifies the reading order of integer and
> -  * decimal part of CPU temp for atomic reads. If bit == 0,
> -  * reading integer part triggers latching of the decimal part,
> -  * so integer part should be read first. If bit == 1, read
> -  * order should be reversed.
> -  */
> - err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
> - if (err < 0)
> - return err;
> -
> - mutex_lock(>lock);
> - if (err & BIT(SBTSI_CONFIG_READ_ORDER_SHIFT)) {
> - temp_dec = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_DEC);
> - temp_int = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_INT);
> - } else {
> - temp_int = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_INT);
> - temp_dec = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_DEC);
> - }
> - mutex_unlock(>lock);
> + temp_int_reg = SBTSI_REG_TEMP_INT;
> + temp_dec_reg = SBTSI_REG_TEMP_DEC;
>   break;
>   case hwmon_temp_max:
> - mutex_lock(>lock);
> - temp_int = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_HIGH_INT);
> - temp_dec = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_HIGH_DEC);
> - mutex_unlock(>lock);
> + temp_int_reg = SBTSI_REG_TEMP_HIGH_INT;
> + temp_dec_reg = SBTSI_REG_TEMP_HIGH_DEC;
>   break;
>   case hwmon_temp_min:
> - mutex_lock(>lock);
> - temp_int = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_LOW_INT);
> - temp_dec = i2c_smbus_read_byte_data(data->client, 
> SBTSI_REG_TEMP_LOW_DEC);
> - mutex_unlock(>lock);
> + temp_int_reg = SBTSI_REG_TEMP_LOW_INT;
> + temp_dec_reg = SBTSI_REG_TEMP_LOW_DEC;
>   break;
>   default:
>   return -EINVAL;
>   }
>  
> + /*
> +  * ReadOrder bit specifies the reading order of integer and
> +  * decimal part of CPU temp for atomic reads. If bit == 0,
> +  * reading integer part triggers latching of the decimal part,
> +  * so integer part should be read first. If bit == 1, read
> +  * order should be reversed.
> +  */
> + err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
> + if (err < 0)
> + return err;
> +
> + mutex_lock(>lock);
> + if (err & BIT(SBTSI_CONFIG_READ_ORDER_SHIFT)) {
> + temp_dec = i2c_smbus_read_byte_data(data->client, temp_dec_reg);
> + temp_int = i2c_smbus_read_byte_data(data->client, temp_int_reg);
> + } else {
> + temp_int = i2c_smbus_read_byte_data(data->client, temp_int_reg);
> + temp_dec = i2c_smbus_read_byte_data(data->client, temp_dec_reg);
> + }
> + mutex_unlock(>lock);
>  
>   if (temp_int < 0)
>   return temp_int;
> 

Re: [PATCH v3 08/15] arm64: socfpga: merge Agilex and N5X into ARCH_INTEL_SOCFPGA

[Guenter Roeck]

On Thu, Mar 11, 2021 at 04:25:38PM +0100, Krzysztof Kozlowski wrote:
> Agilex, N5X and Stratix 10 share all quite similar arm64 hard cores and
> SoC-part.  Up to a point that N5X uses the same DTSI as Agilex.  From
> the Linux kernel point of view these are flavors of the same
> architecture so there is no need for three top-level arm64
> architectures.  Simplify this by merging all three architectures into
> ARCH_INTEL_SOCFPGA and dropping the other ARCH* arm64 Kconfig entries.
> 
> The side effect is that the INTEL_STRATIX10_SERVICE will now be
> available for both 32-bit and 64-bit Intel SoCFPGA, even though it is
> used only for 64-bit.

Did you try to compile, say, arm:allmodconfig with this patch applied ?
Because for me that results in:

In file included from :
drivers/firmware/stratix10-rsu.c: In function 'rsu_status_callback':
include/linux/compiler_types.h:320:38: error:
call to '__compiletime_assert_177' declared with attribute error:
FIELD_GET: type of reg too small for mask

and lots of similar errors.

Guenter

> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  arch/arm64/Kconfig.platforms   | 21 -
>  arch/arm64/boot/dts/intel/Makefile |  6 +++---
>  arch/arm64/configs/defconfig   |  3 +--
>  drivers/clk/Makefile   |  2 --
>  drivers/clk/socfpga/Kconfig|  4 ++--
>  drivers/firmware/Kconfig   |  2 +-
>  drivers/fpga/Kconfig   |  2 +-
>  drivers/reset/Kconfig  |  2 +-
>  8 files changed, 13 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index ecab67a1afb8..ce50dd129eec 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -8,16 +8,6 @@ config ARCH_ACTIONS
>   help
> This enables support for the Actions Semiconductor S900 SoC family.
>  
> -config ARCH_AGILEX
> - bool "Intel's Agilex SoCFPGA Family"
> - help
> -   This enables support for Intel's Agilex SoCFPGA Family.
> -
> -config ARCH_N5X
> - bool "Intel's eASIC N5X SoCFPGA Family"
> - help
> -   This enables support for Intel's eASIC N5X SoCFPGA Family.
> -
>  config ARCH_SUNXI
>   bool "Allwinner sunxi 64-bit SoC Family"
>   select ARCH_HAS_RESET_CONTROLLER
> @@ -254,14 +244,11 @@ config ARCH_SEATTLE
>   help
> This enables support for AMD Seattle SOC Family
>  
> -config ARCH_STRATIX10
> - bool "Altera's Stratix 10 SoCFPGA Family"
> - select ARCH_INTEL_SOCFPGA
> - help
> -   This enables support for Altera's Stratix 10 SoCFPGA Family.
> -
>  config ARCH_INTEL_SOCFPGA
> - bool
> + bool "Intel's SoCFPGA ARMv8 Families"
> + help
> +   This enables support for Intel's SoCFPGA ARMv8 families:
> +   Stratix 10 (ex. Altera), Agilex and eASIC N5X.
>  
>  config ARCH_SYNQUACER
>   bool "Socionext SynQuacer SoC Family"
> diff --git a/arch/arm64/boot/dts/intel/Makefile 
> b/arch/arm64/boot/dts/intel/Makefile
> index 3a052540605b..0b5477442263 100644
> --- a/arch/arm64/boot/dts/intel/Makefile
> +++ b/arch/arm64/boot/dts/intel/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> -dtb-$(CONFIG_ARCH_AGILEX) += socfpga_agilex_socdk.dtb \
> -  socfpga_agilex_socdk_nand.dtb
> +dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += socfpga_agilex_socdk.dtb \
> + socfpga_agilex_socdk_nand.dtb \
> + socfpga_n5x_socdk.dtb
>  dtb-$(CONFIG_ARCH_KEEMBAY) += keembay-evm.dtb
> -dtb-$(CONFIG_ARCH_N5X) += socfpga_n5x_socdk.dtb
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index d612f633b771..cf8a3009b858 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -28,7 +28,6 @@ CONFIG_KALLSYMS_ALL=y
>  # CONFIG_COMPAT_BRK is not set
>  CONFIG_PROFILING=y
>  CONFIG_ARCH_ACTIONS=y
> -CONFIG_ARCH_AGILEX=y
>  CONFIG_ARCH_SUNXI=y
>  CONFIG_ARCH_ALPINE=y
>  CONFIG_ARCH_BCM2835=y
> @@ -50,7 +49,7 @@ CONFIG_ARCH_RENESAS=y
>  CONFIG_ARCH_ROCKCHIP=y
>  CONFIG_ARCH_S32=y
>  CONFIG_ARCH_SEATTLE=y
> -CONFIG_ARCH_STRATIX10=y
> +CONFIG_ARCH_INTEL_SOCFPGA=y
>  CONFIG_ARCH_SYNQUACER=y
>  CONFIG_ARCH_TEGRA=y
>  CONFIG_ARCH_SPRD=y
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 1e29e5ad107a..96802294d35a 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -105,8 +105,6 @@ obj-$(CONFIG_ARCH_ROCKCHIP)   += rockchip/
>  obj-$(CONFIG_COMMON_CLK_SAMSUNG) += samsung/
>  obj-$(CONFIG_CLK_SIFIVE) += sifive/
>  obj-$(CONFIG_ARCH_INTEL_SOCFPGA) += socfpga/
> -obj-$(CONFIG_ARCH_AGILEX)+= socfpga/
> -obj-$(CONFIG_ARCH_N5X)   += socfpga/
>  obj-$(CONFIG_PLAT_SPEAR) += spear/
>  obj-y+= sprd/
>  obj-$(CONFIG_ARCH_STI)   += st/
> diff --git a/drivers/clk/socfpga/Kconfig b/drivers/clk/socfpga/Kconfig
> index 

Re: [PATCH 11/13] lib: add fast path for find_first_*_bit() and find_last_bit()

[Guenter Roeck]

On Mon, Mar 15, 2021 at 06:54:22PM -0700, Yury Norov wrote:
> Similarly to bitmap functions, users would benefit if we'll handle
> a case of small-size bitmaps that fit into a single word.
> 
> While here, move the find_last_bit() declaration to bitops/find.h
> where other find_*_bit() functions sit.
> 
> Signed-off-by: Yury Norov 
> ---
>  include/asm-generic/bitops/find.h | 50 ---
>  include/linux/bitops.h| 12 
>  lib/find_bit.c| 12 
>  3 files changed, 52 insertions(+), 22 deletions(-)
> 
> diff --git a/include/asm-generic/bitops/find.h 
> b/include/asm-generic/bitops/find.h
> index 4148c74a1e4d..8d818b304869 100644
> --- a/include/asm-generic/bitops/find.h
> +++ b/include/asm-generic/bitops/find.h
> @@ -5,6 +5,9 @@
>  extern unsigned long _find_next_bit(const unsigned long *addr1,
>   const unsigned long *addr2, unsigned long nbits,
>   unsigned long start, unsigned long invert, unsigned long le);
> +extern unsigned long _find_first_bit(const unsigned long *addr, unsigned 
> long size);
> +extern unsigned long _find_first_zero_bit(const unsigned long *addr, 
> unsigned long size);
> +extern unsigned long _find_last_bit(const unsigned long *addr, unsigned long 
> size);
>  
>  #ifndef find_next_bit
>  /**
> @@ -102,8 +105,17 @@ unsigned long find_next_zero_bit(const unsigned long 
> *addr, unsigned long size,
>   * Returns the bit number of the first set bit.
>   * If no bits are set, returns @size.
>   */
> -extern unsigned long find_first_bit(const unsigned long *addr,
> - unsigned long size);
> +static inline
> +unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
> +{
> + if (small_const_nbits(size)) {
> + unsigned long val = *addr & BITS_FIRST(size - 1);
> +
> + return val ? __ffs(val) : size;

This patch results in:

include/asm-generic/bitops/find.h: In function 'find_last_bit':
include/asm-generic/bitops/find.h:164:16: error: implicit declaration of 
function '__fls'; did you mean '__ffs'?

and:

./include/asm-generic/bitops/__fls.h: At top level:
./include/asm-generic/bitops/__fls.h:13:38: error: conflicting types for '__fls'

when building scripts/mod/devicetable-offsets.o.

Seen with h8300 builds.

Guenter

> + }
> +
> + return _find_first_bit(addr, size);
> +}
>  
>  /**
>   * find_first_zero_bit - find the first cleared bit in a memory region
> @@ -113,8 +125,17 @@ extern unsigned long find_first_bit(const unsigned long 
> *addr,
>   * Returns the bit number of the first cleared bit.
>   * If no bits are zero, returns @size.
>   */
> -extern unsigned long find_first_zero_bit(const unsigned long *addr,
> -  unsigned long size);
> +static inline
> +unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long 
> size)
> +{
> + if (small_const_nbits(size)) {
> + unsigned long val = *addr | ~BITS_FIRST(size - 1);
> +
> + return val == ~0UL ? size : ffz(val);
> + }
> +
> + return _find_first_zero_bit(addr, size);
> +}
>  #else /* CONFIG_GENERIC_FIND_FIRST_BIT */
>  
>  #ifndef find_first_bit
> @@ -126,6 +147,27 @@ extern unsigned long find_first_zero_bit(const unsigned 
> long *addr,
>  
>  #endif /* CONFIG_GENERIC_FIND_FIRST_BIT */
>  
> +#ifndef find_last_bit
> +/**
> + * find_last_bit - find the last set bit in a memory region
> + * @addr: The address to start the search at
> + * @size: The number of bits to search
> + *
> + * Returns the bit number of the last set bit, or size.
> + */
> +static inline
> +unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
> +{
> + if (small_const_nbits(size)) {
> + unsigned long val = *addr & BITS_FIRST(size - 1);
> +
> + return val ? __fls(val) : size;
> + }
> +
> + return _find_last_bit(addr, size);
> +}
> +#endif
> +
>  /**
>   * find_next_clump8 - find next 8-bit clump with set bits in a memory region
>   * @clump: location to store copy of found clump
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index a5a48303b0f1..26bf15e6cd35 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -286,17 +286,5 @@ static __always_inline void __assign_bit(long nr, 
> volatile unsigned long *addr,
>  })
>  #endif
>  
> -#ifndef find_last_bit
> -/**
> - * find_last_bit - find the last set bit in a memory region
> - * @addr: The address to start the search at
> - * @size: The number of bits to search
> - *
> - * Returns the bit number of the last set bit, or size.
> - */
> -extern unsigned long find_last_bit(const unsigned long *addr,
> -unsigned long size);
> -#endif
> -
>  #endif /* __KERNEL__ */
>  #endif
> diff --git a/lib/find_bit.c b/lib/find_bit.c
> index 2470ae390f3c..e2c301d28568 100644
> --- a/lib/find_bit.c
> +++ b/lib/find_bit.c
> @@ -75,7 +75,7 @@ 

Re: [PATCH v2 07/10] watchdog: npcm: Add support for WPCM450

[Guenter Roeck]

On 4/6/21 5:09 AM, Jonathan Neuschäfer wrote:
> Add a compatible string for WPCM450, which has essentially the same
> watchdog mechanism as NPCM750.
> 
> Signed-off-by: Jonathan Neuschäfer 

Acked-by: Guenter Roeck 

> ---
> 
> v2:
> - Added patch description
> ---
>  drivers/watchdog/npcm_wdt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index 765577f11c8db..28a24caa2627c 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -229,6 +229,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
> 
>  #ifdef CONFIG_OF
>  static const struct of_device_id npcm_wdt_match[] = {
> + {.compatible = "nuvoton,wpcm450-wdt"},
>   {.compatible = "nuvoton,npcm750-wdt"},
>   {},
>  };
> --
> 2.30.2
> 

Re: [PATCH] softdog: make pretimeout available when SOFT_WATCHDOG_PRETIMEOUT enabled

[Guenter Roeck]

On 4/6/21 2:44 AM, Wang Qing wrote:
> Although softdog supports pretimeout, there is no way to set pretimeout, 
> so pretimeout will never be processed in softdog_ping(). 
> 
This is wrong. The pretimeout can be set using WDIOC_SETPRETIMEOUT, as with
every other driver supporting it. There is no need for a module parameter.

Guenter

> Here add the configuration mechanism for pretimeout and the default value
> is 1 second, so when CONFIG_SOFT_WATCHDOG_PRETIMEOUT is enabled, the 
> pretimeout function defaults available.
> 
> Signed-off-by: Wang Qing 
> ---
>  drivers/watchdog/softdog.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 7a10962..79e52791
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -35,6 +35,14 @@ MODULE_PARM_DESC(soft_margin,
>   "Watchdog soft_margin in seconds. (0 < soft_margin < 65536, default="
>   __MODULE_STRING(TIMER_MARGIN) ")");
>  
> +#ifdef CONFIG_SOFT_WATCHDOG_PRETIMEOUT
> +#define PRE_TIMER_MARGIN 1   /* Default is 1 seconds */
> +static unsigned int soft_pretimeout = PRE_TIMER_MARGIN;  /* in seconds */
> +module_param(soft_pretimeout, uint, 0);
> +MODULE_PARM_DESC(soft_pretimeout,
> + "Watchdog soft_pretimeout in seconds. (0 < soft_pretimeout < 
> soft_margin, default=1)");
> +#endif
> +
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout,
> @@ -177,6 +185,9 @@ static struct watchdog_device softdog_dev = {
>   .min_timeout = 1,
>   .max_timeout = 65535,
>   .timeout = TIMER_MARGIN,
> +#ifdef CONFIG_SOFT_WATCHDOG_PRETIMEOUT
> + .pretimeout = PRE_TIMER_MARGIN,
> +#endif
>  };
>  
>  static int __init softdog_init(void)
> 

[PATCH] pcnet32: Use pci_resource_len to validate PCI resource

[Guenter Roeck]

pci_resource_start() is not a good indicator to determine if a PCI
resource exists or not, since the resource may start at address 0.
This is seen when trying to instantiate the driver in qemu for riscv32
or riscv64.

pci :00:01.0: reg 0x10: [io  0x-0x001f]
pci :00:01.0: reg 0x14: [mem 0x-0x001f]
...
pcnet32: card has no PCI IO resources, aborting

Use pci_resouce_len() instead.

Signed-off-by: Guenter Roeck 
---
 drivers/net/ethernet/amd/pcnet32.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amd/pcnet32.c 
b/drivers/net/ethernet/amd/pcnet32.c
index 187b0b9a6e1d..f78daba60b35 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1534,8 +1534,7 @@ pcnet32_probe_pci(struct pci_dev *pdev, const struct 
pci_device_id *ent)
}
pci_set_master(pdev);
 
-   ioaddr = pci_resource_start(pdev, 0);
-   if (!ioaddr) {
+   if (!pci_resource_len(pdev, 0)) {
if (pcnet32_debug & NETIF_MSG_PROBE)
pr_err("card has no PCI IO resources, aborting\n");
err = -ENODEV;
@@ -1548,6 +1547,8 @@ pcnet32_probe_pci(struct pci_dev *pdev, const struct 
pci_device_id *ent)
pr_err("architecture does not support 32bit PCI 
busmaster DMA\n");
goto err_disable_dev;
}
+
+   ioaddr = pci_resource_start(pdev, 0);
if (!request_region(ioaddr, PCNET32_TOTAL_SIZE, "pcnet32_probe_pci")) {
if (pcnet32_debug & NETIF_MSG_PROBE)
pr_err("io address range already allocated\n");
-- 
2.17.1