2020-03-06 12:48:43

by Serge Semin

[permalink] [raw]
Subject: [PATCH 00/22] mips: Prepare MIPS-arch code for Baikal-T1 SoC support

From: Serge Semin <[email protected]>

This is a first patchset of a series of about 25 ones, which are intended to
add the full Baikal-T1 SoC [1] support to the Linux kernel. Since they will
concern various kernel subsystems, I decided to split the whole work up into
the patchesets in accordance with the subsystems/devices their changes are
introduced for. Nearly 2/3 of the work is already done and will be sent out
very soon. While the rest of the changes specifically related to the fast-speed
interfaces (DW 12G PHY, PCIe, SATA, xGBE, GMAC, USB, DDRC, IC) are still in
refactoring and preparation for integration into the mainline kernel. Hopefully
I'll finish them up in the next two-three months, and submit them straight
away.

Getting back to this patchset. As the subject states this is intended to
prepare the MIPS-arch and generic kernel code for further Baikal-T1 SoC
platform support integration (note the Baikal-T1 SoC platform code will be
submitted last after the whole series of patchsets as a closure of the
submission process). First of all the patchset starts with a set of changes
to the dt-bindings kernel concerning MIPS CPC and CDMM nodes as being described
by the trivial device bindings. In addition we updated the vendors prefix
schema with Baikal Electronics JSC prefix so being further committed device
drivers would be correctly accepted by the checkpatch-script.

Then I found out that dtc-compiler integrated into the kernel doesn't support
all the possible values of the 'reg'-property used to define the i2c-slave
devices address. It may have 10-bits and own-slave flags, so the compiler has
to deal with them. Even though the patch might be better to be integrated into
the mainline dtc repo I decided to send it to the kernel first.

While I was working with the MIPS architecture code, I discovered, that there
is a bug in the Coherency Manager v2 error causes declaration and the errors
handler lacked of CM2 L2 ECC/parity errors support. So the fixes are here in
the patchset.

Baikal-T1 SoC is based on the MIPS P5600 Warrior IP-core, which itself has
MIPS32 Release 5 architecture. Even though on ISA level it doesn't differ much
from the MIPS32 Release 2 release, there are still some peculiarities, which
make it's justified to add the direct MIPS32r5 support into the kernel (see
the specific patch for details). In addition seeing there is more than one
real chip based on the MIPS P5600 core on the market, it would be good to have
the direct P5600 CPU config in the MIPS-arch.

There were some issues we discovered while were working with MIPS-arch code.
So the cleanups and fixes are introduced in this patchset. First of all the
Write-Merge CPU feature hasn't been handled in a generic way. Even if a
platform defined the writecombine flag as _CACHE_UNCACHED_ACCELERATED, the
feature might have been disabled in the CP0 register. We either enable it or
leave it as is in accordance with the knowledge of whether the corresponding
platform really supports it. Secondly Memory Accessibility Attribute Registers
(MAAR) haven't been properly initialized when Extended Physical Address (XPA)
mode was enabled. Thirdly since some of the platforms may have a very strict
limitations on the IO-memory access instructions. For instance Baikal-T1 SoC
IO-memory can be accessed by the lw/sw instructions only. In this case
for early-printk and CPS-debug code we suggest to use the instructions in
accordance with the UART-registers offset (lb/sb if offset = 0, lh/sh
if offset = 1 and so on). Fourthly in case if CPUFREQ feature is enabled
and frequency of the CPU is changed by the reference clock alteration, we
must make sure that MIPS r4k timer related services are properly updated
when CPU-frequency changes. It concerns udelay lpj adjustment, MIPS timer
clockevent frequency update. In addition when CPU reference frequency changes
it isn't recommended to use the timer as clocksource at all, since currently
the subsystem isn't tolerant to the unstable clock sources. So in this case
we suggest to use the r4k timer for clocksourcing only as a last resort.
Fifthly we discovered a bug in a method of CPUFREQ boost feature enable
procedure and fixed it in one of the patches within this patchset. And finally
there are a few fixups/cleanups we suggest to integrate into the MIPS FDC
and CDMM related code (see the patches for details).

This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
commit 98d54f81e36b ("Linux 5.6-rc4").

[1] http://www.baikalelectronics.com/products/168/

P.S. Sorry for the previous emails burst. I forgot to start the series with
this cover-letter and the corporate smtp broke the transmission in the middle
anyway. Please don't pay attention to them. Here is the proper emails
resubmission.

Signed-off-by: Serge Semin <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Cc: Maxim Kaurkin <[email protected]>
Cc: Pavel Parkhomenko <[email protected]>
Cc: Ramil Zaripov <[email protected]>
Cc: Ekaterina Skachko <[email protected]>
Cc: Vadim Vlasov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
cc: Tony Lindgren <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Serge Semin (22):
dt-bindings: Permit platform devices in the trivial-devices bindings
dt-bindings: Add MIPS CPC controller as a trivial devices
dt-bindings: Add MIPS CDMM controller as a trivial device
dt-bindings: Add vendor prefix for Baikal Electronics, JSC
mips: cm: Fix an invalid error code of INTVN_*_ERR
mips: cm: Add L2 ECC/parity errors reporting
mips: Add MIPS32 Release 5 support
mips: Add MIPS Warrior P5600 support
mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs
mips: Add CP0 Write Merge config support
mips: Add CONFIG/CONFIG6 reg fields macro
mips: MAAR: Use more precise address mask
mips: MAAR: Add XPA mode support
mips: early_printk_8250: Use offset-sized IO-mem accessors
mips: Use offset-sized IO-mem accessors in CPS debug printout
mips: cdmm: Add mti,mips-cdmm dtb node support
bus: cdmm: Add MIPS R5 arch support
tty: mips_ejtag_fdc: Mark expected switch fall-through
mips: Add udelay lpj numbers adjustment
mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU
cpufreq: Return zero on success in boost sw setting

.../bindings/power/mti,mips-cpc.txt | 8 ---
.../devicetree/bindings/trivial-devices.yaml | 12 ++--
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
arch/mips/Kconfig | 62 ++++++++++++++--
arch/mips/Makefile | 2 +
arch/mips/include/asm/asmmacro.h | 18 ++---
arch/mips/include/asm/compiler.h | 5 ++
arch/mips/include/asm/cpu-features.h | 34 ++++++---
arch/mips/include/asm/cpu-info.h | 2 +-
arch/mips/include/asm/cpu-type.h | 6 +-
arch/mips/include/asm/cpu.h | 11 +--
arch/mips/include/asm/fpu.h | 4 +-
arch/mips/include/asm/hazards.h | 8 ++-
arch/mips/include/asm/maar.h | 17 ++++-
arch/mips/include/asm/mipsregs.h | 33 ++++++++-
arch/mips/include/asm/module.h | 4 ++
arch/mips/include/asm/stackframe.h | 2 +-
arch/mips/include/asm/switch_to.h | 8 +--
arch/mips/kernel/cevt-r4k.c | 44 ++++++++++++
arch/mips/kernel/cps-vec-ns16550.S | 18 ++++-
arch/mips/kernel/cpu-probe.c | 60 ++++++++++++++++
arch/mips/kernel/csrc-r4k.c | 4 ++
arch/mips/kernel/early_printk_8250.c | 34 ++++++++-
arch/mips/kernel/entry.S | 6 +-
arch/mips/kernel/mips-cm.c | 66 +++++++++++++++--
arch/mips/kernel/proc.c | 2 +
arch/mips/kernel/r4k_fpu.S | 14 ++--
arch/mips/kernel/spram.c | 4 +-
arch/mips/kernel/time.c | 70 +++++++++++++++++++
arch/mips/kvm/vz.c | 6 +-
arch/mips/lib/csum_partial.S | 6 +-
arch/mips/mm/c-r4k.c | 7 +-
arch/mips/mm/init.c | 8 ++-
arch/mips/mm/sc-mips.c | 7 +-
drivers/bus/Kconfig | 2 +-
drivers/bus/mips_cdmm.c | 15 ++++
drivers/cpufreq/cpufreq.c | 2 +-
drivers/tty/mips_ejtag_fdc.c | 1 +
38 files changed, 529 insertions(+), 85 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/power/mti,mips-cpc.txt

--
2.25.1


2020-03-10 01:03:49

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH 00/22] mips: Prepare MIPS-arch code for Baikal-T1 SoC support

On Fri, Mar 06, 2020 at 03:46:43PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> This is a first patchset of a series of about 25 ones, which are intended to
> add the full Baikal-T1 SoC [1] support to the Linux kernel. Since they will
> concern various kernel subsystems, I decided to split the whole work up into
> the patchesets in accordance with the subsystems/devices their changes are
> introduced for. Nearly 2/3 of the work is already done and will be sent out
> very soon. While the rest of the changes specifically related to the fast-speed
> interfaces (DW 12G PHY, PCIe, SATA, xGBE, GMAC, USB, DDRC, IC) are still in
> refactoring and preparation for integration into the mainline kernel. Hopefully
> I'll finish them up in the next two-three months, and submit them straight
> away.
>
> Getting back to this patchset. As the subject states this is intended to
> prepare the MIPS-arch and generic kernel code for further Baikal-T1 SoC
> platform support integration (note the Baikal-T1 SoC platform code will be
> submitted last after the whole series of patchsets as a closure of the
> submission process). First of all the patchset starts with a set of changes
> to the dt-bindings kernel concerning MIPS CPC and CDMM nodes as being described
> by the trivial device bindings. In addition we updated the vendors prefix
> schema with Baikal Electronics JSC prefix so being further committed device
> drivers would be correctly accepted by the checkpatch-script.
>
> Then I found out that dtc-compiler integrated into the kernel doesn't support
> all the possible values of the 'reg'-property used to define the i2c-slave
> devices address. It may have 10-bits and own-slave flags, so the compiler has
> to deal with them. Even though the patch might be better to be integrated into
> the mainline dtc repo I decided to send it to the kernel first.
>
> While I was working with the MIPS architecture code, I discovered, that there
> is a bug in the Coherency Manager v2 error causes declaration and the errors
> handler lacked of CM2 L2 ECC/parity errors support. So the fixes are here in
> the patchset.
>
> Baikal-T1 SoC is based on the MIPS P5600 Warrior IP-core, which itself has
> MIPS32 Release 5 architecture. Even though on ISA level it doesn't differ much
> from the MIPS32 Release 2 release, there are still some peculiarities, which
> make it's justified to add the direct MIPS32r5 support into the kernel (see
> the specific patch for details). In addition seeing there is more than one
> real chip based on the MIPS P5600 core on the market, it would be good to have
> the direct P5600 CPU config in the MIPS-arch.
>
> There were some issues we discovered while were working with MIPS-arch code.
> So the cleanups and fixes are introduced in this patchset. First of all the
> Write-Merge CPU feature hasn't been handled in a generic way. Even if a
> platform defined the writecombine flag as _CACHE_UNCACHED_ACCELERATED, the
> feature might have been disabled in the CP0 register. We either enable it or
> leave it as is in accordance with the knowledge of whether the corresponding
> platform really supports it. Secondly Memory Accessibility Attribute Registers
> (MAAR) haven't been properly initialized when Extended Physical Address (XPA)
> mode was enabled. Thirdly since some of the platforms may have a very strict
> limitations on the IO-memory access instructions. For instance Baikal-T1 SoC
> IO-memory can be accessed by the lw/sw instructions only. In this case
> for early-printk and CPS-debug code we suggest to use the instructions in
> accordance with the UART-registers offset (lb/sb if offset = 0, lh/sh
> if offset = 1 and so on). Fourthly in case if CPUFREQ feature is enabled
> and frequency of the CPU is changed by the reference clock alteration, we
> must make sure that MIPS r4k timer related services are properly updated
> when CPU-frequency changes. It concerns udelay lpj adjustment, MIPS timer
> clockevent frequency update. In addition when CPU reference frequency changes
> it isn't recommended to use the timer as clocksource at all, since currently
> the subsystem isn't tolerant to the unstable clock sources. So in this case
> we suggest to use the r4k timer for clocksourcing only as a last resort.
> Fifthly we discovered a bug in a method of CPUFREQ boost feature enable
> procedure and fixed it in one of the patches within this patchset. And finally
> there are a few fixups/cleanups we suggest to integrate into the MIPS FDC
> and CDMM related code (see the patches for details).
>
> This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
> commit 98d54f81e36b ("Linux 5.6-rc4").
>
> [1] http://www.baikalelectronics.com/products/168/
>
> P.S. Sorry for the previous emails burst. I forgot to start the series with
> this cover-letter and the corporate smtp broke the transmission in the middle
> anyway. Please don't pay attention to them. Here is the proper emails
> resubmission.
>
> Signed-off-by: Serge Semin <[email protected]>
> Signed-off-by: Alexey Malahov <[email protected]>
> Cc: Maxim Kaurkin <[email protected]>
> Cc: Pavel Parkhomenko <[email protected]>
> Cc: Ramil Zaripov <[email protected]>
> Cc: Ekaterina Skachko <[email protected]>
> Cc: Vadim Vlasov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Frank Rowand <[email protected]>
> cc: Tony Lindgren <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Jiri Slaby <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Serge Semin (22):
> dt-bindings: Permit platform devices in the trivial-devices bindings
> dt-bindings: Add MIPS CPC controller as a trivial devices
> dt-bindings: Add MIPS CDMM controller as a trivial device
> dt-bindings: Add vendor prefix for Baikal Electronics, JSC
> mips: cm: Fix an invalid error code of INTVN_*_ERR
> mips: cm: Add L2 ECC/parity errors reporting
> mips: Add MIPS32 Release 5 support
> mips: Add MIPS Warrior P5600 support
> mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs
> mips: Add CP0 Write Merge config support
> mips: Add CONFIG/CONFIG6 reg fields macro
> mips: MAAR: Use more precise address mask
> mips: MAAR: Add XPA mode support
> mips: early_printk_8250: Use offset-sized IO-mem accessors
> mips: Use offset-sized IO-mem accessors in CPS debug printout
> mips: cdmm: Add mti,mips-cdmm dtb node support
> bus: cdmm: Add MIPS R5 arch support
> tty: mips_ejtag_fdc: Mark expected switch fall-through
> mips: Add udelay lpj numbers adjustment
> mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
> mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU
> cpufreq: Return zero on success in boost sw setting
>
> .../bindings/power/mti,mips-cpc.txt | 8 ---
> .../devicetree/bindings/trivial-devices.yaml | 12 ++--
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> arch/mips/Kconfig | 62 ++++++++++++++--
> arch/mips/Makefile | 2 +
> arch/mips/include/asm/asmmacro.h | 18 ++---
> arch/mips/include/asm/compiler.h | 5 ++
> arch/mips/include/asm/cpu-features.h | 34 ++++++---
> arch/mips/include/asm/cpu-info.h | 2 +-
> arch/mips/include/asm/cpu-type.h | 6 +-
> arch/mips/include/asm/cpu.h | 11 +--
> arch/mips/include/asm/fpu.h | 4 +-
> arch/mips/include/asm/hazards.h | 8 ++-
> arch/mips/include/asm/maar.h | 17 ++++-
> arch/mips/include/asm/mipsregs.h | 33 ++++++++-
> arch/mips/include/asm/module.h | 4 ++
> arch/mips/include/asm/stackframe.h | 2 +-
> arch/mips/include/asm/switch_to.h | 8 +--
> arch/mips/kernel/cevt-r4k.c | 44 ++++++++++++
> arch/mips/kernel/cps-vec-ns16550.S | 18 ++++-
> arch/mips/kernel/cpu-probe.c | 60 ++++++++++++++++
> arch/mips/kernel/csrc-r4k.c | 4 ++
> arch/mips/kernel/early_printk_8250.c | 34 ++++++++-
> arch/mips/kernel/entry.S | 6 +-
> arch/mips/kernel/mips-cm.c | 66 +++++++++++++++--
> arch/mips/kernel/proc.c | 2 +
> arch/mips/kernel/r4k_fpu.S | 14 ++--
> arch/mips/kernel/spram.c | 4 +-
> arch/mips/kernel/time.c | 70 +++++++++++++++++++
> arch/mips/kvm/vz.c | 6 +-
> arch/mips/lib/csum_partial.S | 6 +-
> arch/mips/mm/c-r4k.c | 7 +-
> arch/mips/mm/init.c | 8 ++-
> arch/mips/mm/sc-mips.c | 7 +-
> drivers/bus/Kconfig | 2 +-
> drivers/bus/mips_cdmm.c | 15 ++++
> drivers/cpufreq/cpufreq.c | 2 +-
> drivers/tty/mips_ejtag_fdc.c | 1 +
> 38 files changed, 529 insertions(+), 85 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/power/mti,mips-cpc.txt
>
> --
> 2.25.1
>

Folks,

It appears our corporate email server changes the Message-Id field of
messages passing through it. Due to that the emails threading gets to be
broken. I'll resubmit the properly structured v2 patchset as soon as our system
administrator fixes the problem. Sorry for the inconvenience caused by it.

Regards,
-Sergey

2020-05-06 17:45:12

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 00/20] mips: Prepare MIPS-arch code for Baikal-T1 SoC support

From: Serge Semin <[email protected]>

This is a first patchset of a series of about 25 ones, which are intended to
add the full Baikal-T1 SoC [1] support to the Linux kernel. Since they will
concern various kernel subsystems, I decided to split the whole work up into
the patchesets in accordance with the subsystems/devices their changes are
introduced for. Nearly 2/3 of the work is already done and will be sent out
very soon. While the rest of the changes specifically related to the fast-speed
interfaces (DW 12G PHY, PCIe, SATA, xGBE, GMAC, USB, DDRC, IC) are still in
refactoring and preparation for integration into the mainline kernel. Hopefully
I'll finish them up in the next two-three months, and submit them straight
away.

Getting back to this patchset. As the subject states this is intended to
prepare the MIPS-arch and generic kernel code for further Baikal-T1 SoC
platform support integration (note the Baikal-T1 SoC platform code will be
submitted last after the whole series of patchsets as a closure of the
submission process). First of all the patchset starts with a set of changes
to the dt-bindings kernel concerning MIPS CPC and CDMM nodes to make them
being represented by dt schemas. In addition we updated the vendors prefix
schema with Baikal Electronics JSC prefix so being further committed
vendor-specific device drivers would be correctly accepted by the
checkpatch script.

While I was working with the MIPS architecture code, I discovered, that there
is a bug in the Coherency Manager v2 error causes declaration and the errors
handler lacked of CM2 L2 ECC/parity errors support. So the fixes are here in
the patchset.

Baikal-T1 SoC is based on the MIPS P5600 Warrior IP-core, which itself has
MIPS32 Release 5 architecture. Even though on ISA level it doesn't differ much
from the MIPS32 Release 2 release, there are still some peculiarities, which
make it's justified to add the direct MIPS32r5 support into the kernel (see
the specific patch for details). In addition seeing there is more than one
real chip based on the MIPS P5600 core on the market, it would be good to have
the direct P5600 CPU config in the MIPS-arch.

There were some issues we discovered while were working with MIPS-arch code.
So the cleanups and fixes are introduced in this patchset. First of all the
Write-Merge CPU feature hasn't been handled in a generic way. Even if a
platform defined the writecombine flag as _CACHE_UNCACHED_ACCELERATED, the
feature might have been disabled in the CP0 register. We either enable it or
leave it as is in accordance with the knowledge of whether the corresponding
platform really supports it. Secondly Memory Accessibility Attribute Registers
(MAAR) haven't been properly initialized when Extended Physical Address (XPA)
mode was enabled. Thirdly since some of the platforms may have a very strict
limitations on the IO-memory access instructions. For instance Baikal-T1 SoC
IO-memory can be accessed by the lw/sw instructions only. In this case
for early-printk and CPS-debug code we suggest to use the instructions in
accordance with the UART-registers offset (lb/sb if offset = 0, lh/sh
if offset = 1 and so on). Fourthly in case if CPUFREQ feature is enabled
and frequency of the CPU is changed by the reference clock alteration, we
must make sure that MIPS r4k timer related services are properly updated
when CPU-frequency changes. It concerns udelay lpj adjustment, MIPS timer
clockevent frequency update. In addition when CPU reference frequency changes
it isn't recommended to use the timer as clocksource at all, since currently
the subsystem isn't tolerant to the unstable clock sources. So in this case
we suggest to use the r4k timer for clocksourcing only as a last resort.
Fifthly we discovered a bug in a method of CPUFREQ boost feature enable
procedure and fixed it in one of the patches within this patchset. And finally
there are a few fixups/cleanups we suggest to integrate into the MIPS FDC
and CDMM related code (see the patches for details).

This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4

[1] http://www.baikalelectronics.com/products/168/

Changelog v2:
- Replace "be" vendor prefix with "baikal" one.
- Remove patches adding the platform devices to the trivial-devices.yaml
bindings file (Rob nacked it).
- Add yaml-based bindings file for MIPS CDMM dt-node.
- Convert mti,mips-cpc to DT schema.
- Use a shorter summary describing the bindings modification patches.
- Rearrange the SoBs with adding Alexey' co-development tag.
- Lowercase the hex numbers in the dt-bindings.
- Fix author and SoB emails mismatch in the patch of adding the Baikal
Electronis JSC prefix to the dt schema of the prefixes.
- Remove patch "tty: mips_ejtag_fdc: Mark expected switch fall-through" from
the patchset as being already applied to the Greg' tty-next branch.
- Our corporate email server doesn't change Message-Id anymore, so the patchset
is resubmitted being in the cover-letter-threaded format.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Maxim Kaurkin <[email protected]>
Cc: Pavel Parkhomenko <[email protected]>
Cc: Ramil Zaripov <[email protected]>
Cc: Ekaterina Skachko <[email protected]>
Cc: Vadim Vlasov <[email protected]>
Cc: Alexey Kolotnikov <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Olof Johansson <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Serge Semin (20):
dt-bindings: power: Convert mti,mips-cpc to DT schema
dt-bindings: bus: Add MIPS CDMM controller
dt-bindings: Add vendor prefix for Baikal Electronics, JSC
mips: cm: Fix an invalid error code of INTVN_*_ERR
mips: cm: Add L2 ECC/parity errors reporting
mips: Add MIPS32 Release 5 support
mips: Add MIPS Warrior P5600 support
mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs
mips: Add CP0 Write Merge config support
mips: Add CONFIG/CONFIG6/Cause reg fields macro
mips: MAAR: Use more precise address mask
mips: MAAR: Add XPA mode support
mips: early_printk_8250: Use offset-sized IO-mem accessors
mips: Use offset-sized IO-mem accessors in CPS debug printout
mips: cdmm: Add mti,mips-cdmm dtb node support
bus: cdmm: Add MIPS R5 arch support
mips: Add udelay lpj numbers adjustment
mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU
cpufreq: Return zero on success in boost sw setting

.../bindings/bus/mti,mips-cdmm.yaml | 35 ++++++++++
.../bindings/power/mti,mips-cpc.txt | 8 ---
.../bindings/power/mti,mips-cpc.yaml | 35 ++++++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
arch/mips/Kconfig | 62 ++++++++++++++--
arch/mips/Makefile | 2 +
arch/mips/include/asm/asmmacro.h | 18 ++---
arch/mips/include/asm/compiler.h | 5 ++
arch/mips/include/asm/cpu-features.h | 34 ++++++---
arch/mips/include/asm/cpu-info.h | 2 +-
arch/mips/include/asm/cpu-type.h | 6 +-
arch/mips/include/asm/cpu.h | 11 +--
arch/mips/include/asm/fpu.h | 4 +-
arch/mips/include/asm/hazards.h | 8 ++-
arch/mips/include/asm/maar.h | 17 ++++-
arch/mips/include/asm/mipsregs.h | 34 ++++++++-
arch/mips/include/asm/stackframe.h | 2 +-
arch/mips/include/asm/switch_to.h | 8 +--
arch/mips/include/asm/vermagic.h | 4 ++
arch/mips/kernel/cevt-r4k.c | 44 ++++++++++++
arch/mips/kernel/cps-vec-ns16550.S | 18 ++++-
arch/mips/kernel/cpu-probe.c | 60 ++++++++++++++++
arch/mips/kernel/csrc-r4k.c | 4 ++
arch/mips/kernel/early_printk_8250.c | 34 ++++++++-
arch/mips/kernel/entry.S | 6 +-
arch/mips/kernel/mips-cm.c | 66 +++++++++++++++--
arch/mips/kernel/proc.c | 2 +
arch/mips/kernel/r4k_fpu.S | 14 ++--
arch/mips/kernel/spram.c | 4 +-
arch/mips/kernel/time.c | 70 +++++++++++++++++++
arch/mips/kvm/vz.c | 6 +-
arch/mips/lib/csum_partial.S | 6 +-
arch/mips/mm/c-r4k.c | 7 +-
arch/mips/mm/init.c | 8 ++-
arch/mips/mm/sc-mips.c | 7 +-
drivers/bus/Kconfig | 2 +-
drivers/bus/mips_cdmm.c | 15 ++++
drivers/cpufreq/cpufreq.c | 2 +-
38 files changed, 591 insertions(+), 81 deletions(-)
create mode 100644 Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml
delete mode 100644 Documentation/devicetree/bindings/power/mti,mips-cpc.txt
create mode 100644 Documentation/devicetree/bindings/power/mti,mips-cpc.yaml

--
2.25.1

2020-05-06 17:45:19

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 02/20] dt-bindings: bus: Add MIPS CDMM controller

From: Serge Semin <[email protected]>

It's a Common Device Memory Map controller embedded into the MIPS IP
cores, which dts node is supposed to have compatible and reg properties.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: [email protected]

---

Changelog v2:
- Lowercase the example hex'es.
---
.../bindings/bus/mti,mips-cdmm.yaml | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml

diff --git a/Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml b/Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml
new file mode 100644
index 000000000000..d28d65ae57b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/mti,mips-cdmm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPS Common Device Memory Map
+
+description: |
+ Defines a location of the MIPS Common Device Memory Map registers.
+
+maintainers:
+ - James Hogan <[email protected]>
+
+properties:
+ compatible:
+ const: mti,mips-cdmm
+
+ reg:
+ description: |
+ Base address and size of an unoccupied memory region, which will be
+ used to map the MIPS CDMM registers block.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ cdmm@1bde8000 {
+ compatible = "mti,mips-cdmm";
+ reg = <0 0x1bde8000 0 0x8000>;
+ };
+...
--
2.25.1

2020-05-06 17:45:30

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

From: Serge Semin <[email protected]>

Add "BAIKAL ELECTRONICS, JSC" to the list of devicetree vendor prefixes
as "baikal".

Website: http://www.baikalelectronics.com

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: [email protected]

---

Changelog v2:
- Fix author and SoB emails mismatch.
- Add 'baikal' vendor prefix instead of ambiguous 'be'.
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index d3891386d671..674c0d07c0ad 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -139,6 +139,8 @@ patternProperties:
description: Azoteq (Pty) Ltd
"^azw,.*":
description: Shenzhen AZW Technology Co., Ltd.
+ "^baikal,.*":
+ description: BAIKAL ELECTRONICS, JSC
"^bananapi,.*":
description: BIPAI KEJI LIMITED
"^beacon,.*":
--
2.25.1

2020-05-06 17:45:53

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 04/20] mips: cm: Fix an invalid error code of INTVN_*_ERR

From: Serge Semin <[email protected]>

Commit 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache
errors") adds cm2_causes[] array with map of error type ID and
pointers to the short description string. There is a mistake in
the table, since according to MIPS32 manual CM2_ERROR_TYPE = {17,18}
correspond to INTVN_WR_ERR and INTVN_RD_ERR, while the table
claims they have {0x17,0x18} codes. This is obviously hex-dec
copy-paste bug. Moreover codes {0x18 - 0x1a} indicate L2 ECC errors.

Fixes: 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache errors")
Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/mips-cm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index cdb93ed91cde..361bfc91a0e6 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -119,9 +119,9 @@ static char *cm2_causes[32] = {
"COH_RD_ERR", "MMIO_WR_ERR", "MMIO_RD_ERR", "0x07",
"0x08", "0x09", "0x0a", "0x0b",
"0x0c", "0x0d", "0x0e", "0x0f",
- "0x10", "0x11", "0x12", "0x13",
- "0x14", "0x15", "0x16", "INTVN_WR_ERR",
- "INTVN_RD_ERR", "0x19", "0x1a", "0x1b",
+ "0x10", "INTVN_WR_ERR", "INTVN_RD_ERR", "0x13",
+ "0x14", "0x15", "0x16", "0x17",
+ "0x18", "0x19", "0x1a", "0x1b",
"0x1c", "0x1d", "0x1e", "0x1f"
};

--
2.25.1

2020-05-06 17:46:09

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 08/20] mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs

From: Serge Semin <[email protected]>

Commit 1aeba347b3a9 ("MIPS: Hardcode cpu_has_mips* where target ISA
allows") updated the cpu_has_mips* macro to be replaced with a constant
expression where it's possible. By mistake it wasn't done correctly
for cpu_has_mips64r1/cpu_has_mips64r2 macro. They are defined to
be replaced with conditional expression __isa_range_or_flag(), which
means either ISA revision being within the range or the corresponding
CPU options flag was set at the probe stage or both being true at the
same time. But the ISA level value doesn't indicate whether the ISA is
MIPS32 or MIPS64. Due to this if we select MIPS32r1 - MIPS32r5
architectures the __isa_range() macro will activate the
cpu_has_mips64rX flags, which is incorrect. In order to fix the
problem we added a new macro __isa_range_and_flag() and use it to
define the cpu_has_mips64r1/cpu_has_mips64r2 flags.

Fixes: 1aeba347b3a9 ("MIPS: Hardcode cpu_has_mips* where target ISA allows")
Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/cpu-features.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index e2f31bd6363b..7e22b9c1e279 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -64,6 +64,8 @@
((MIPS_ISA_REV >= (ge)) && (MIPS_ISA_REV < (lt)))
#define __isa_range_or_flag(ge, lt, flag) \
(__isa_range(ge, lt) || ((MIPS_ISA_REV < (lt)) && __isa(flag)))
+#define __isa_range_and_flag(ge, lt, flag) \
+ (__isa_range(ge, lt) && __isa(flag))

/*
* SMP assumption: Options of CPU 0 are a superset of all processors.
@@ -291,10 +293,10 @@
# define cpu_has_mips32r6 __isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
#endif
#ifndef cpu_has_mips64r1
-# define cpu_has_mips64r1 __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1)
+# define cpu_has_mips64r1 __isa_range_and_flag(1, 6, MIPS_CPU_ISA_M64R1)
#endif
#ifndef cpu_has_mips64r2
-# define cpu_has_mips64r2 __isa_range_or_flag(2, 6, MIPS_CPU_ISA_M64R2)
+# define cpu_has_mips64r2 __isa_range_and_flag(2, 6, MIPS_CPU_ISA_M64R2)
#endif
#ifndef cpu_has_mips64r6
# define cpu_has_mips64r6 __isa_ge_and_flag(6, MIPS_CPU_ISA_M64R6)
--
2.25.1

2020-05-06 17:46:22

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro

From: Serge Semin <[email protected]>

There are bit fields which persist in the MIPS CONFIG and CONFIG6
registers, but haven't been described in the generic mipsregs.h
header so far. In particular, the generic CONFIG bitfields are
BE - endian mode, BM - burst mode, SB - SimpleBE, OCP interface mode
indicator, UDI - user-defined "CorExtend" instructions, DSP - data
scratch pad RAM present, ISP - instruction scratch pad RAM present,
etc. The core-specific CONFIG6 bitfields are JRCD - jump register
cache prediction disable, R6 - MIPSr6 extensions enable, IFUPerfCtl -
IFU performance control, SPCD - sleep state performance counter, DLSB -
disable load/store bonding. A new exception code reported in the
ExcCode field of the Cause register: 30 - Parity/ECC error exception
happened on either fetch, load or cache refill. Lets add them to the
mipsregs.h header to be used in future platform code, which have them
utilized.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/mipsregs.h | 19 +++++++++++++++++++
arch/mips/kernel/spram.c | 4 ++--
2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index b1c761279b13..039ebd913f00 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -468,6 +468,7 @@
#define EXCCODE_THREAD 25 /* Thread exceptions (MT) */
#define EXCCODE_DSPDIS 26 /* DSP disabled exception */
#define EXCCODE_GE 27 /* Virtualized guest exception (VZ) */
+#define EXCCODE_CACHEERR 30 /* Parity/ECC occured on a core */

/* Implementation specific trap codes used by MIPS cores */
#define MIPS_EXCCODE_TLBPAR 16 /* TLB parity error exception */
@@ -563,9 +564,17 @@
#define MIPS_CONF_MT_FTLB (_ULCAST_(4) << 7)
#define MIPS_CONF_AR (_ULCAST_(7) << 10)
#define MIPS_CONF_AT (_ULCAST_(3) << 13)
+#define MIPS_CONF_BE (_ULCAST_(1) << 15)
+#define MIPS_CONF_BM (_ULCAST_(1) << 16)
#define MIPS_CONF_MM (_ULCAST_(3) << 17)
#define MIPS_CONF_MM_SYSAD (_ULCAST_(1) << 17)
#define MIPS_CONF_MM_FULL (_ULCAST_(2) << 17)
+#define MIPS_CONF_SB (_ULCAST_(1) << 21)
+#define MIPS_CONF_UDI (_ULCAST_(1) << 22)
+#define MIPS_CONF_DSP (_ULCAST_(1) << 23)
+#define MIPS_CONF_ISP (_ULCAST_(1) << 24)
+#define MIPS_CONF_KU (_ULCAST_(3) << 25)
+#define MIPS_CONF_K23 (_ULCAST_(3) << 28)
#define MIPS_CONF_M (_ULCAST_(1) << 31)

/*
@@ -677,9 +686,19 @@
#define MIPS_CONF5_CV (_ULCAST_(1) << 29)
#define MIPS_CONF5_K (_ULCAST_(1) << 30)

+/* Jump register cache prediction disable */
+#define MIPS_CONF6_JRCD (_ULCAST_(1) << 0)
+/* MIPSr6 extensions enable */
+#define MIPS_CONF6_R6 (_ULCAST_(1) << 2)
+/* IFU Performance Control */
+#define MIPS_CONF6_IFUPERFCTL (_ULCAST_(3) << 10)
#define MIPS_CONF6_SYND (_ULCAST_(1) << 13)
+/* Sleep state performance counter disable */
+#define MIPS_CONF6_SPCD (_ULCAST_(1) << 14)
/* proAptiv FTLB on/off bit */
#define MIPS_CONF6_FTLBEN (_ULCAST_(1) << 15)
+/* Disable load/store bonding */
+#define MIPS_CONF6_DLSB (_ULCAST_(1) << 21)
/* Loongson-3 FTLB on/off bit */
#define MIPS_CONF6_FTLBDIS (_ULCAST_(1) << 22)
/* FTLB probability bits */
diff --git a/arch/mips/kernel/spram.c b/arch/mips/kernel/spram.c
index 26d355462ace..d5d96214cce5 100644
--- a/arch/mips/kernel/spram.c
+++ b/arch/mips/kernel/spram.c
@@ -209,11 +209,11 @@ void spram_config(void)
case CPU_P6600:
config0 = read_c0_config();
/* FIXME: addresses are Malta specific */
- if (config0 & (1<<24)) {
+ if (config0 & MIPS_CONF_ISP) {
probe_spram("ISPRAM", 0x1c000000,
&ispram_load_tag, &ispram_store_tag);
}
- if (config0 & (1<<23))
+ if (config0 & MIPS_CONF_DSP)
probe_spram("DSPRAM", 0x1c100000,
&dspram_load_tag, &dspram_store_tag);
}
--
2.25.1

2020-05-06 17:46:23

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 09/20] mips: Add CP0 Write Merge config support

From: Serge Semin <[email protected]>

CP0 config register may indicate whether write-through merging
is allowed. Currently there are two types of the merging available:
SysAD Valid and Full modes. Whether each of them are supported by
the core is implementation dependent. Moreover whether the ability
to change the mode also depends on the chip family instance. Taking
into account all of this we created a dedicated mm_config() method
to detect and enable merging if it's supported. It is called for
MIPS-type processors at CPU-probe stage and attempts to detect whether
the write merging is available. If it's known to be supported and
switchable, then switch on the full mode. Otherwise just perform the
CP0.Config.MM field analysis.

In addition there are platforms like InterAptiv/ProAptiv, which do have
the MM bit field set by default, but having write-through cacheing
unsupported makes write-merging also unsupported. In this case we just
ignore the MM field value.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/cpu-features.h | 8 +++++
arch/mips/include/asm/cpu.h | 4 ++-
arch/mips/include/asm/mipsregs.h | 3 ++
arch/mips/kernel/cpu-probe.c | 48 ++++++++++++++++++++++++++++
4 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index 7e22b9c1e279..2b818f2036d0 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -630,6 +630,14 @@
# endif
#endif

+#ifndef cpu_has_mm_sysad
+# define cpu_has_mm_sysad __opt(MIPS_CPU_MM_SYSAD)
+#endif
+
+#ifndef cpu_has_mm_full
+# define cpu_has_mm_full __opt(MIPS_CPU_MM_FULL)
+#endif
+
/*
* Guest capabilities
*/
diff --git a/arch/mips/include/asm/cpu.h b/arch/mips/include/asm/cpu.h
index 9bae99b568c9..191529ef0d05 100644
--- a/arch/mips/include/asm/cpu.h
+++ b/arch/mips/include/asm/cpu.h
@@ -417,7 +417,9 @@ enum cpu_type_enum {
#define MIPS_CPU_MT_PER_TC_PERF_COUNTERS \
BIT_ULL(56) /* CPU has perf counters implemented per TC (MIPSMT ASE) */
#define MIPS_CPU_MMID BIT_ULL(57) /* CPU supports MemoryMapIDs */
-#define MIPS_CPU_MAC_2008_ONLY BIT_ULL(58) /* CPU Only support MAC2008 Fused multiply-add instruction */
+#define MIPS_CPU_MM_SYSAD BIT_ULL(58) /* CPU supports write-through SysAD Valid merge */
+#define MIPS_CPU_MM_FULL BIT_ULL(59) /* CPU supports write-through full merge */
+#define MIPS_CPU_MAC_2008_ONLY BIT_ULL(60) /* CPU Only support MAC2008 Fused multiply-add instruction */

/*
* CPU ASE encodings
diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 796fe47cfd17..b1c761279b13 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -563,6 +563,9 @@
#define MIPS_CONF_MT_FTLB (_ULCAST_(4) << 7)
#define MIPS_CONF_AR (_ULCAST_(7) << 10)
#define MIPS_CONF_AT (_ULCAST_(3) << 13)
+#define MIPS_CONF_MM (_ULCAST_(3) << 17)
+#define MIPS_CONF_MM_SYSAD (_ULCAST_(1) << 17)
+#define MIPS_CONF_MM_FULL (_ULCAST_(2) << 17)
#define MIPS_CONF_M (_ULCAST_(1) << 31)

/*
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index a2dafef2df45..ad298d34304f 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -667,6 +667,52 @@ static int set_ftlb_enable(struct cpuinfo_mips *c, enum ftlb_flags flags)
return 0;
}

+static int mm_config(struct cpuinfo_mips *c)
+{
+ unsigned int config0, update, mm;
+
+ config0 = read_c0_config();
+ mm = config0 & MIPS_CONF_MM;
+
+ /*
+ * It's implementation dependent what type of write-merge is supported
+ * and whether it can be enabled/disabled. If it is settable lets make
+ * the merging allowed by default. Some platforms might have
+ * write-through caching unsupported. In this case just ignore the
+ * CP0.Config.MM bit field value.
+ */
+ switch (c->cputype) {
+ case CPU_24K:
+ case CPU_34K:
+ case CPU_74K:
+ case CPU_P5600:
+ case CPU_P6600:
+ c->options |= MIPS_CPU_MM_FULL;
+ update = MIPS_CONF_MM_FULL;
+ break;
+ case CPU_1004K:
+ case CPU_1074K:
+ case CPU_INTERAPTIV:
+ case CPU_PROAPTIV:
+ mm = 0;
+ /* fall through */
+ default:
+ update = 0;
+ break;
+ }
+
+ if (update) {
+ config0 = (config0 & ~MIPS_CONF_MM) | update;
+ write_c0_config(config0);
+ } else if (mm == MIPS_CONF_MM_SYSAD) {
+ c->options |= MIPS_CPU_MM_SYSAD;
+ } else if (mm == MIPS_CONF_MM_FULL) {
+ c->options |= MIPS_CPU_MM_FULL;
+ }
+
+ return 0;
+}
+
static inline unsigned int decode_config0(struct cpuinfo_mips *c)
{
unsigned int config0;
@@ -1758,6 +1804,8 @@ static inline void cpu_probe_mips(struct cpuinfo_mips *c, unsigned int cpu)

spram_config();

+ mm_config(c);
+
switch (__get_cpu_type(c->cputype)) {
case CPU_M5150:
case CPU_P5600:
--
2.25.1

2020-05-06 17:46:37

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 07/20] mips: Add MIPS Warrior P5600 support

From: Serge Semin <[email protected]>

This is a MIPS32 Release 5 based IP core with XPA, EVA, dual/quad issue
exec pipes, MMU with two-levels TLB, UCA, MSA, MDU core level features
and system level features like up to six P5600 calculation cores, CM2
with L2 cache, IOCU/IOMMU (though might be unused depending on the
system-specific IP core configuration), GIC, CPC, virtualisation module,
eJTAG and PDtrace.

As being MIPS32 Release 5 based core it provides all the features
available by the CPU_MIPS32_R5 config, while adding a few more like
UCA attribute support, availability of CPU-freq (by means of L2/CM
clock ratio setting), EI/VI GIC modes detection at runtime.

In addition to this if P5600 architecture is enabled modern GNU GCC
provides a specific tuning for P5600 processors with respect to the
classic MIPS32 Release 5. First of all branch-likely avoidance is
activated only when the code is compiled with the speed optimization
(avoidance is always enabled for the pure MIPS32 Release 5
architecture). Secondly the madd/msub avoidance is enabled since
madd/msub utilization isn't profitable due to overhead of getting the
result out of the HI/LO registers. Multiply-accumulate instructions are
activated and utilized together with the necessary code reorder when
multiply-add/multiply-subtract statements are met. Finally load/store
bonding is activated by default. All of these optimizations may make
the code relatively faster than if just MIP32 release 5 architecture
was requested.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/Kconfig | 38 +++++++++++++++++++++++++++-----
arch/mips/Makefile | 1 +
arch/mips/include/asm/vermagic.h | 2 ++
3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 55c3dbfea336..e3b780a389a9 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1665,6 +1665,28 @@ config CPU_MIPS64_R6
family, are based on a MIPS64r6 processor. If you own an older
processor, you probably need to select MIPS64r1 or MIPS64r2 instead.

+config CPU_P5600
+ bool "MIPS Warrior P5600"
+ depends on SYS_HAS_CPU_P5600
+ select CPU_HAS_PREFETCH
+ select CPU_SUPPORTS_32BIT_KERNEL
+ select CPU_SUPPORTS_HIGHMEM
+ select CPU_SUPPORTS_MSA
+ select CPU_SUPPORTS_UNCACHED_ACCELERATED
+ select CPU_SUPPORTS_CPUFREQ
+ select CPU_MIPSR2_IRQ_VI
+ select CPU_MIPSR2_IRQ_EI
+ select HAVE_KVM
+ select MIPS_O32_FP64_SUPPORT
+ help
+ Choose this option to build a kernel for MIPS Warrior P5600 CPU.
+ It's based on MIPS32r5 ISA with XPA, EVA, dual/quad issue exec pipes,
+ MMU with two-levels TLB, UCA, MSA, MDU core level features and system
+ level features like up to six P5600 calculation cores, CM2 with L2
+ cache, IOCU/IOMMU (though might be unused depending on the system-
+ specific IP core configuration), GIC, CPC, virtualisation module,
+ eJTAG and PDtrace.
+
config CPU_R3000
bool "R3000"
depends on SYS_HAS_CPU_R3000
@@ -1841,7 +1863,8 @@ endchoice
config CPU_MIPS32_3_5_FEATURES
bool "MIPS32 Release 3.5 Features"
depends on SYS_HAS_CPU_MIPS32_R3_5
- depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6
+ depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6 || \
+ CPU_P5600
help
Choose this option to build a kernel for release 2 or later of the
MIPS32 architecture including features from the 3.5 release such as
@@ -1861,7 +1884,7 @@ config CPU_MIPS32_3_5_EVA
config CPU_MIPS32_R5_FEATURES
bool "MIPS32 Release 5 Features"
depends on SYS_HAS_CPU_MIPS32_R5
- depends on CPU_MIPS32_R2 || CPU_MIPS32_R5
+ depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_P5600
help
Choose this option to build a kernel for release 2 or later of the
MIPS32 architecture including features from release 5 such as
@@ -2016,6 +2039,10 @@ config SYS_HAS_CPU_MIPS64_R6
bool
select ARCH_HAS_SYNC_DMA_FOR_CPU if DMA_NONCOHERENT

+config SYS_HAS_CPU_P5600
+ bool
+ select ARCH_HAS_SYNC_DMA_FOR_CPU if DMA_NONCOHERENT
+
config SYS_HAS_CPU_R3000
bool

@@ -2100,7 +2127,7 @@ endmenu
config CPU_MIPS32
bool
default y if CPU_MIPS32_R1 || CPU_MIPS32_R2 || CPU_MIPS32_R5 || \
- CPU_MIPS32_R6
+ CPU_MIPS32_R6 || CPU_P5600

config CPU_MIPS64
bool
@@ -2122,7 +2149,7 @@ config CPU_MIPSR2

config CPU_MIPSR5
bool
- default y if CPU_MIPS32_R5
+ default y if CPU_MIPS32_R5 || CPU_P5600
select CPU_HAS_RIXI
select CPU_HAS_DIEI if !CPU_DIEI_BROKEN
select MIPS_SPRAM
@@ -2733,7 +2760,8 @@ config RELOCATABLE
bool "Relocatable kernel"
depends on SYS_SUPPORTS_RELOCATABLE
depends on CPU_MIPS32_R2 || CPU_MIPS64_R2 || CPU_MIPS32_R5 || \
- CPU_MIPS32_R6 || CPU_MIPS64_R6 || CAVIUM_OCTEON_SOC
+ CPU_MIPS32_R6 || CPU_MIPS64_R6 || CPU_P5600 || \
+ CAVIUM_OCTEON_SOC
help
This builds a kernel image that retains relocation information
so it can be loaded someplace besides the default 1MB.
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 9172fb0f630b..264dead560f4 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -176,6 +176,7 @@ cflags-$(CONFIG_CPU_MIPS32_R6) += -march=mips32r6 -Wa,--trap -modd-spreg
cflags-$(CONFIG_CPU_MIPS64_R1) += -march=mips64 -Wa,--trap
cflags-$(CONFIG_CPU_MIPS64_R2) += -march=mips64r2 -Wa,--trap
cflags-$(CONFIG_CPU_MIPS64_R6) += -march=mips64r6 -Wa,--trap
+cflags-$(CONFIG_CPU_P5600) += -march=p5600 -Wa,--trap -modd-spreg
cflags-$(CONFIG_CPU_R5000) += -march=r5000 -Wa,--trap
cflags-$(CONFIG_CPU_R5500) += $(call cc-option,-march=r5500,-march=r5000) \
-Wa,--trap
diff --git a/arch/mips/include/asm/vermagic.h b/arch/mips/include/asm/vermagic.h
index 5a0e739f597a..d03f97350f91 100644
--- a/arch/mips/include/asm/vermagic.h
+++ b/arch/mips/include/asm/vermagic.h
@@ -48,6 +48,8 @@
#define MODULE_PROC_FAMILY "LOONGSON64 "
#elif defined CONFIG_CPU_CAVIUM_OCTEON
#define MODULE_PROC_FAMILY "OCTEON "
+#elif defined CONFIG_CPU_P5600
+#define MODULE_PROC_FAMILY "P5600 "
#elif defined CONFIG_CPU_XLR
#define MODULE_PROC_FAMILY "XLR "
#elif defined CONFIG_CPU_XLP
--
2.25.1

2020-05-06 17:47:01

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

From: Serge Semin <[email protected]>

Recent commit e61a41256edf ("cpufreq: dev_pm_qos_update_request() can
return 1 on success") fixed a problem when active policies traverse
was falsely stopped due to invalidly treating the non-zero return value
from freq_qos_update_request() method as an error. Yes, that function
can return positive values if the requested update actually took place.
The current problem is that the returned value is then passed to the
return cell of the cpufreq_boost_set_sw() (set_boost callback) method.
This value is then also analyzed for being non-zero, which is also
treated as having an error. As a result during the boost activation
we'll get an error returned while having the QOS frequency update
successfully performed. Fix this by returning a negative value from the
cpufreq_boost_set_sw() if actual error was encountered and zero
otherwise treating any positive values as the successful operations
completion.

Fixes: 18c49926c4bf ("cpufreq: Add QoS requests for userspace constraints")
Signed-off-by: Serge Semin <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/cpufreq/cpufreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 045f9fe157ce..5870cdca88cf 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state)
break;
}

- return ret;
+ return ret < 0 ? ret : 0;
}

int cpufreq_boost_trigger_state(int state)
--
2.25.1

2020-05-06 17:47:18

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 19/20] mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU

From: Serge Semin <[email protected]>

Due to being embedded into the CPU cores MIPS count/compare timer
frequency is changed together with the CPU clocks alteration.
In case if frequency really changes the kernel clockevent framework
must be notified, otherwise the kernel timers won't work correctly.
Fix this by calling clockevents_update_freq() for each r4k clockevent
handlers registered per available CPUs.

Traditionally MIPS r4k-clock are clocked with CPU frequency divided by 2.
But this isn't true for some of the platforms. Due to this we have to save
the basic CPU frequency, so then use it to scale the initial timer
frequency (mips_hpt_frequency) and pass the updated value further to the
clockevent framework.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/cevt-r4k.c | 44 +++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
index 17a9cbb8b3df..f5b72fb7d5ee 100644
--- a/arch/mips/kernel/cevt-r4k.c
+++ b/arch/mips/kernel/cevt-r4k.c
@@ -8,6 +8,7 @@
*/
#include <linux/clockchips.h>
#include <linux/interrupt.h>
+#include <linux/cpufreq.h>
#include <linux/percpu.h>
#include <linux/smp.h>
#include <linux/irq.h>
@@ -250,6 +251,49 @@ unsigned int __weak get_c0_compare_int(void)
return MIPS_CPU_IRQ_BASE + cp0_compare_irq;
}

+#ifdef CONFIG_CPU_FREQ
+
+static unsigned long mips_ref_freq;
+
+static int cpufreq_callback(struct notifier_block *nb,
+ unsigned long val, void *data)
+{
+ struct cpufreq_freqs *freq = data;
+ struct clock_event_device *cd;
+ unsigned long rate;
+ int cpu;
+
+ if (!mips_ref_freq)
+ mips_ref_freq = freq->old;
+
+ if (val == CPUFREQ_POSTCHANGE) {
+ rate = cpufreq_scale(mips_hpt_frequency, mips_ref_freq,
+ freq->new);
+
+ for_each_cpu(cpu, freq->policy->cpus) {
+ cd = &per_cpu(mips_clockevent_device, cpu);
+
+ clockevents_update_freq(cd, rate);
+ }
+ }
+
+ return 0;
+}
+
+static struct notifier_block cpufreq_notifier = {
+ .notifier_call = cpufreq_callback,
+};
+
+static int __init register_cpufreq_notifier(void)
+{
+ return cpufreq_register_notifier(&cpufreq_notifier,
+ CPUFREQ_TRANSITION_NOTIFIER);
+
+}
+core_initcall(register_cpufreq_notifier);
+
+#endif /* !CONFIG_CPU_FREQ */
+
int r4k_clockevent_init(void)
{
unsigned long flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED;
--
2.25.1

2020-05-06 17:47:32

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support

From: Serge Semin <[email protected]>

Since having and mapping the CDMM block is platform specific, then
instead of just returning a zero-address, lets make the default CDMM
base address search method (mips_cdmm_phys_base()) to do something
useful. For instance to find the address in a dedicated dtb-node in
order to support of-based platforms by default.

Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Olof Johansson <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/bus/mips_cdmm.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/drivers/bus/mips_cdmm.c b/drivers/bus/mips_cdmm.c
index 1b14256376d2..7faa8c049f07 100644
--- a/drivers/bus/mips_cdmm.c
+++ b/drivers/bus/mips_cdmm.c
@@ -16,6 +16,8 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/smp.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
#include <asm/cdmm.h>
#include <asm/hazards.h>
#include <asm/mipsregs.h>
@@ -337,9 +339,22 @@ static phys_addr_t mips_cdmm_cur_base(void)
* Picking a suitable physical address at which to map the CDMM region is
* platform specific, so this weak function can be overridden by platform
* code to pick a suitable value if none is configured by the bootloader.
+ * By default this method tries to find a CDMM-specific node in the system
+ * dtb. Note that this won't work for early serial console.
*/
phys_addr_t __weak mips_cdmm_phys_base(void)
{
+ struct device_node *np;
+ struct resource res;
+ int err;
+
+ np = of_find_compatible_node(NULL, NULL, "mti,mips-cdmm");
+ if (np) {
+ err = of_address_to_resource(np, 0, &res);
+ if (!err)
+ return res.start;
+ }
+
return 0;
}

--
2.25.1

2020-05-06 17:47:49

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 17/20] mips: Add udelay lpj numbers adjustment

From: Serge Semin <[email protected]>

Loops-per-jiffies is a special number which represents a number of
noop-loop cycles per CPU-scheduler quantum - jiffies. As you
understand aside from CPU-specific implementation it depends on
the CPU frequency. So when a platform has the CPU frequency fixed,
we have no problem and the current udelay interface will work
just fine. But as soon as CPU-freq driver is enabled and the cores
frequency changes, we'll end up with distorted udelay's. In order
to fix this we have to accordinly adjust the per-CPU udelay_val
(the same as the global loops_per_jiffy) number. This can be done
in the CPU-freq transition event handler. We subscribe to that event
in the MIPS arch time-inititalization method.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/time.c | 70 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)

diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 37e9413a393d..ce89e18af024 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -18,12 +18,82 @@
#include <linux/smp.h>
#include <linux/spinlock.h>
#include <linux/export.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>

#include <asm/cpu-features.h>
#include <asm/cpu-type.h>
#include <asm/div64.h>
#include <asm/time.h>

+#ifdef CONFIG_CPU_FREQ
+
+static DEFINE_PER_CPU(unsigned long, pcp_lpj_ref);
+static DEFINE_PER_CPU(unsigned long, pcp_lpj_ref_freq);
+static unsigned long glb_lpj_ref;
+static unsigned long glb_lpj_ref_freq;
+
+static int cpufreq_callback(struct notifier_block *nb,
+ unsigned long val, void *data)
+{
+ struct cpufreq_freqs *freq = data;
+ struct cpumask *cpus = freq->policy->cpus;
+ unsigned long lpj;
+ int cpu;
+
+ /*
+ * Skip lpj numbers adjustment if the CPU-freq transition is safe for
+ * the loops delay. (Is this possible?)
+ */
+ if (freq->flags & CPUFREQ_CONST_LOOPS)
+ return NOTIFY_OK;
+
+ /* Save the initial values of the lpjes for future scaling. */
+ if (!glb_lpj_ref) {
+ glb_lpj_ref = boot_cpu_data.udelay_val;
+ glb_lpj_ref_freq = freq->old;
+
+ for_each_online_cpu(cpu) {
+ per_cpu(pcp_lpj_ref, cpu) =
+ cpu_data[cpu].udelay_val;
+ per_cpu(pcp_lpj_ref_freq, cpu) = freq->old;
+ }
+ }
+
+ /*
+ * Adjust global lpj variable and per-CPU udelay_val number in
+ * accordance with the new CPU frequency.
+ */
+ if ((val == CPUFREQ_PRECHANGE && freq->old < freq->new) ||
+ (val == CPUFREQ_POSTCHANGE && freq->old > freq->new)) {
+ loops_per_jiffy = cpufreq_scale(glb_lpj_ref,
+ glb_lpj_ref_freq,
+ freq->new);
+
+ for_each_cpu(cpu, cpus) {
+ lpj = cpufreq_scale(per_cpu(pcp_lpj_ref, cpu),
+ per_cpu(pcp_lpj_ref_freq, cpu),
+ freq->new);
+ cpu_data[cpu].udelay_val = (unsigned int)lpj;
+ }
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block cpufreq_notifier = {
+ .notifier_call = cpufreq_callback,
+};
+
+static int __init register_cpufreq_notifier(void)
+{
+ return cpufreq_register_notifier(&cpufreq_notifier,
+ CPUFREQ_TRANSITION_NOTIFIER);
+}
+core_initcall(register_cpufreq_notifier);
+
+#endif /* !CONFIG_CPU_FREQ */
+
/*
* forward reference
*/
--
2.25.1

2020-05-06 17:48:02

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support

From: Serge Semin <[email protected]>

CDMM may be available not only MIPS R2 architectures, but also in
newer MIPS R5 chips. For instance our P5600 chip has one. Lets mark
the CDMM bus being supported for that MIPS arch too.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Olof Johansson <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/bus/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 6d4e4497b59b..971c07bc92d4 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -58,7 +58,7 @@ config IMX_WEIM

config MIPS_CDMM
bool "MIPS Common Device Memory Map (CDMM) Driver"
- depends on CPU_MIPSR2
+ depends on CPU_MIPSR2 || CPU_MIPSR5
help
Driver needed for the MIPS Common Device Memory Map bus in MIPS
cores. This bus is for per-CPU tightly coupled devices such as the
--
2.25.1

2020-05-06 17:48:12

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 06/20] mips: Add MIPS32 Release 5 support

From: Serge Semin <[email protected]>

There are five MIPS32/64 architecture releases currently available:
from 1 to 6 except fourth one, which was intentionally skipped.
Three of them can be called as major: 1st, 2nd and 6th, that not only
have some system level alterations, but also introduced significant
core/ISA level updates. The rest of the MIPS architecture releases are
minor.

Even though they don't have as much ISA/system/core level changes
as the major ones with respect to the previous releases, they still
provide a set of updates (I'd say they were intended to be the
intermediate releases before a major one) that might be useful for the
kernel and user-level code, when activated by the kernel or compiler.
In particular the following features were introduced or ended up being
available at/after MIPS32 Release 5 architecture:
+ the last release of the misaligned memory access instructions,
+ virtualisation - VZ ASE - is optional component of the arch,
+ SIMD - MSA ASE - is optional component of the arch,
+ DSP ASE is optional component of the arch,
+ CP0.Status.FR=1 for CP1.FIR.F64=1 (pure 64-bit FPU general registers)
must be available if FPU is implemented,
+ CP1.FIR.Has2008 support is required so CP1.FCSR.{ABS2008,NAN2008} bits
are available.
+ UFR/UNFR aliases to access CP0.Status.FR from user-space by means of
ctc1/cfc1 instructions (enabled by CP0.Config5.UFR),
+ CP0.COnfig5.LLB=1 and eretnc instruction are implemented to without
accidentally clearing LL-bit when returning from an interrupt,
exception, or error trap,
+ XPA feature together with extended versions of CPx registers is
introduced, which needs to have mfhc0/mthc0 instructions available.

So due to these changes GNU GCC provides an extended instructions set
support for MIPS32 Release 5 by default like eretnc/mfhc0/mthc0. Even
though the architecture alteration isn't that big, it still worth to be
taken into account by the kernel software. Finally we can't deny that
some optimization/limitations might be found in future and implemented
on some level in kernel or compiler. In this case having even
intermediate MIPS architecture releases support would be more than
useful.

So the most of the changes provided by this commit can be split into
either compile- or runtime configs related. The compile-time related
changes are caused by adding the new CONFIG_CPU_MIPS32_R5/CONFIG_CPU_MIPSR5
configs and concern the code activating MIPSR2 or MIPSR6 already
implemented features (like eretnc/LLbit, mthc0/mfhc0). In addition
CPU_HAS_MSA can be now freely enabled for MIPS32 release 5 based
platforms as this is done for CPU_MIPS32_R6 CPUs. The runtime changes
concerns the features which are handled with respect to the MIPS ISA
revision detected at run-time by means of CP0.Config.{AT,AR} bits. Alas
these fields can be used to detect either r1 or r2 or r6 releases.
But since we know which CPUs in fact support the R5 arch, we can manually
set MIPS_CPU_ISA_M32R5 bit of c->isa_level and then use cpu_has_mips32_r5
where it's appropriate.

Since XPA/EVA provide too complex alterationss and to have them used with
MIPS32 Release 2 charged kernels (for compatibility with current platform
configs) they are left to be setup as a separate kernel configs.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
Even without this patch the code contains too many patterns like:
+#if defined(CONFIG_MIPS32R2) || defined(CONFIG_MIPS32R6)
What about switching it to a simpler
+#if CONFIG_TARGET_ISA_REV >= 2 ?
Though I'd prefer this config to be named like CONFIG_CPU_MIPS_REV.
What do you think?

Similarly the pattern like:
(MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
MIPS_CPU_ISA_M32R5 |
MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6)
could be replaced with simpler one:
(MIPS_CPU_ISA_M32 | MIPS_CPU_ISA_M64)
if corresponding macro were available. What do think about adding such?

We also could add CPU_MIPS64_R5 config support here, but I don't think
it's necessary at the moment seeing there is no any real chip ever
produced with that arch. Right?
---
arch/mips/Kconfig | 34 ++++++++++++++++++++++++----
arch/mips/Makefile | 1 +
arch/mips/include/asm/asmmacro.h | 18 ++++++++-------
arch/mips/include/asm/compiler.h | 5 ++++
arch/mips/include/asm/cpu-features.h | 20 +++++++++++-----
arch/mips/include/asm/cpu-info.h | 2 +-
arch/mips/include/asm/cpu-type.h | 6 ++++-
arch/mips/include/asm/cpu.h | 7 +++---
arch/mips/include/asm/fpu.h | 4 ++--
arch/mips/include/asm/hazards.h | 8 ++++---
arch/mips/include/asm/stackframe.h | 2 +-
arch/mips/include/asm/switch_to.h | 8 +++----
arch/mips/include/asm/vermagic.h | 2 ++
arch/mips/kernel/cpu-probe.c | 12 ++++++++++
arch/mips/kernel/entry.S | 6 ++---
arch/mips/kernel/proc.c | 2 ++
arch/mips/kernel/r4k_fpu.S | 14 ++++++------
arch/mips/kvm/vz.c | 6 ++---
arch/mips/lib/csum_partial.S | 6 +++--
arch/mips/mm/c-r4k.c | 7 +++---
arch/mips/mm/sc-mips.c | 7 +++---
21 files changed, 123 insertions(+), 54 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 690718b3701a..55c3dbfea336 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1580,6 +1580,21 @@ config CPU_MIPS32_R2
specific type of processor in your system, choose those that one
otherwise CPU_MIPS32_R1 is a safe bet for any MIPS32 system.

+config CPU_MIPS32_R5
+ bool "MIPS32 Release 5"
+ depends on SYS_HAS_CPU_MIPS32_R5
+ select CPU_HAS_PREFETCH
+ select CPU_SUPPORTS_32BIT_KERNEL
+ select CPU_SUPPORTS_HIGHMEM
+ select CPU_SUPPORTS_MSA
+ select HAVE_KVM
+ select MIPS_O32_FP64_SUPPORT
+ help
+ Choose this option to build a kernel for release 5 or later of the
+ MIPS32 architecture. New MIPS processors, starting with the Warrior
+ family, are based on a MIPS32r5 processor. If you own an older
+ processor, you probably need to select MIPS32r1 or MIPS32r2 instead.
+
config CPU_MIPS32_R6
bool "MIPS32 Release 6"
depends on SYS_HAS_CPU_MIPS32_R6
@@ -1826,7 +1841,7 @@ endchoice
config CPU_MIPS32_3_5_FEATURES
bool "MIPS32 Release 3.5 Features"
depends on SYS_HAS_CPU_MIPS32_R3_5
- depends on CPU_MIPS32_R2 || CPU_MIPS32_R6
+ depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6
help
Choose this option to build a kernel for release 2 or later of the
MIPS32 architecture including features from the 3.5 release such as
@@ -1846,7 +1861,7 @@ config CPU_MIPS32_3_5_EVA
config CPU_MIPS32_R5_FEATURES
bool "MIPS32 Release 5 Features"
depends on SYS_HAS_CPU_MIPS32_R5
- depends on CPU_MIPS32_R2
+ depends on CPU_MIPS32_R2 || CPU_MIPS32_R5
help
Choose this option to build a kernel for release 2 or later of the
MIPS32 architecture including features from release 5 such as
@@ -2084,7 +2099,8 @@ endmenu
#
config CPU_MIPS32
bool
- default y if CPU_MIPS32_R1 || CPU_MIPS32_R2 || CPU_MIPS32_R6
+ default y if CPU_MIPS32_R1 || CPU_MIPS32_R2 || CPU_MIPS32_R5 || \
+ CPU_MIPS32_R6

config CPU_MIPS64
bool
@@ -2104,6 +2120,13 @@ config CPU_MIPSR2
select CPU_HAS_DIEI if !CPU_DIEI_BROKEN
select MIPS_SPRAM

+config CPU_MIPSR5
+ bool
+ default y if CPU_MIPS32_R5
+ select CPU_HAS_RIXI
+ select CPU_HAS_DIEI if !CPU_DIEI_BROKEN
+ select MIPS_SPRAM
+
config CPU_MIPSR6
bool
default y if CPU_MIPS32_R6 || CPU_MIPS64_R6
@@ -2118,6 +2141,7 @@ config TARGET_ISA_REV
int
default 1 if CPU_MIPSR1
default 2 if CPU_MIPSR2
+ default 5 if CPU_MIPSR5
default 6 if CPU_MIPSR6
default 0
help
@@ -2707,7 +2731,9 @@ config NEED_PER_CPU_EMBED_FIRST_CHUNK

config RELOCATABLE
bool "Relocatable kernel"
- depends on SYS_SUPPORTS_RELOCATABLE && (CPU_MIPS32_R2 || CPU_MIPS64_R2 || CPU_MIPS32_R6 || CPU_MIPS64_R6 || CAVIUM_OCTEON_SOC)
+ depends on SYS_SUPPORTS_RELOCATABLE
+ depends on CPU_MIPS32_R2 || CPU_MIPS64_R2 || CPU_MIPS32_R5 || \
+ CPU_MIPS32_R6 || CPU_MIPS64_R6 || CAVIUM_OCTEON_SOC
help
This builds a kernel image that retains relocation information
so it can be loaded someplace besides the default 1MB.
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index e1c44aed8156..9172fb0f630b 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -171,6 +171,7 @@ cflags-$(CONFIG_CPU_R4X00) += -march=r4600 -Wa,--trap
cflags-$(CONFIG_CPU_TX49XX) += -march=r4600 -Wa,--trap
cflags-$(CONFIG_CPU_MIPS32_R1) += -march=mips32 -Wa,--trap
cflags-$(CONFIG_CPU_MIPS32_R2) += -march=mips32r2 -Wa,--trap
+cflags-$(CONFIG_CPU_MIPS32_R5) += -march=mips32r5 -Wa,--trap -modd-spreg
cflags-$(CONFIG_CPU_MIPS32_R6) += -march=mips32r6 -Wa,--trap -modd-spreg
cflags-$(CONFIG_CPU_MIPS64_R1) += -march=mips64 -Wa,--trap
cflags-$(CONFIG_CPU_MIPS64_R2) += -march=mips64r2 -Wa,--trap
diff --git a/arch/mips/include/asm/asmmacro.h b/arch/mips/include/asm/asmmacro.h
index 655f40ddb6d1..86f2323ebe6b 100644
--- a/arch/mips/include/asm/asmmacro.h
+++ b/arch/mips/include/asm/asmmacro.h
@@ -44,7 +44,8 @@
.endm
#endif

-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_MIPSR6)
.macro local_irq_enable reg=t0
ei
irq_enable_hazard
@@ -54,7 +55,7 @@
di
irq_disable_hazard
.endm
-#else
+#else /* !CONFIG_CPU_MIPSR2 && !CONFIG_CPU_MIPSR5 && !CONFIG_CPU_MIPSR6 */
.macro local_irq_enable reg=t0
mfc0 \reg, CP0_STATUS
ori \reg, \reg, 1
@@ -79,7 +80,7 @@
sw \reg, TI_PRE_COUNT($28)
#endif
.endm
-#endif /* CONFIG_CPU_MIPSR2 */
+#endif /* !CONFIG_CPU_MIPSR2 && !CONFIG_CPU_MIPSR5 && !CONFIG_CPU_MIPSR6 */

.macro fpu_save_16even thread tmp=t0
.set push
@@ -131,7 +132,7 @@

.macro fpu_save_double thread status tmp
#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
- defined(CONFIG_CPU_MIPSR6)
+ defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
sll \tmp, \status, 5
bgez \tmp, 10f
fpu_save_16odd \thread
@@ -190,7 +191,7 @@

.macro fpu_restore_double thread status tmp
#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
- defined(CONFIG_CPU_MIPSR6)
+ defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
sll \tmp, \status, 5
bgez \tmp, 10f # 16 register mode?

@@ -200,16 +201,17 @@
fpu_restore_16even \thread \tmp
.endm

-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_MIPSR6)
.macro _EXT rd, rs, p, s
ext \rd, \rs, \p, \s
.endm
-#else /* !CONFIG_CPU_MIPSR2 || !CONFIG_CPU_MIPSR6 */
+#else /* !CONFIG_CPU_MIPSR2 && !CONFIG_CPU_MIPSR5 && !CONFIG_CPU_MIPSR6 */
.macro _EXT rd, rs, p, s
srl \rd, \rs, \p
andi \rd, \rd, (1 << \s) - 1
.endm
-#endif /* !CONFIG_CPU_MIPSR2 || !CONFIG_CPU_MIPSR6 */
+#endif /* !CONFIG_CPU_MIPSR2 && !CONFIG_CPU_MIPSR5 && !CONFIG_CPU_MIPSR6 */

/*
* Temporary until all gas have MT ASE support
diff --git a/arch/mips/include/asm/compiler.h b/arch/mips/include/asm/compiler.h
index f77e99f1722e..a2cb2d2b1c07 100644
--- a/arch/mips/include/asm/compiler.h
+++ b/arch/mips/include/asm/compiler.h
@@ -57,6 +57,11 @@
#define MIPS_ISA_ARCH_LEVEL MIPS_ISA_LEVEL
#define MIPS_ISA_LEVEL_RAW mips64r6
#define MIPS_ISA_ARCH_LEVEL_RAW MIPS_ISA_LEVEL_RAW
+#elif defined(CONFIG_CPU_MIPSR5)
+#define MIPS_ISA_LEVEL "mips64r5"
+#define MIPS_ISA_ARCH_LEVEL MIPS_ISA_LEVEL
+#define MIPS_ISA_LEVEL_RAW mips64r5
+#define MIPS_ISA_ARCH_LEVEL_RAW MIPS_ISA_LEVEL_RAW
#else
/* MIPS64 is a superset of MIPS32 */
#define MIPS_ISA_LEVEL "mips64r2"
diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index de44c92b1c1f..e2f31bd6363b 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -284,6 +284,9 @@
#ifndef cpu_has_mips32r2
# define cpu_has_mips32r2 __isa_range_or_flag(2, 6, MIPS_CPU_ISA_M32R2)
#endif
+#ifndef cpu_has_mips32r5
+# define cpu_has_mips32r5 __isa_range_or_flag(5, 6, MIPS_CPU_ISA_M32R5)
+#endif
#ifndef cpu_has_mips32r6
# define cpu_has_mips32r6 __isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
#endif
@@ -313,19 +316,24 @@
(cpu_has_mips_3 | cpu_has_mips_4_5_64_r2_r6)
#define cpu_has_mips_4_5_64_r2_r6 \
(cpu_has_mips_4_5 | cpu_has_mips64r1 | \
- cpu_has_mips_r2 | cpu_has_mips_r6)
+ cpu_has_mips_r2 | cpu_has_mips_r5 | \
+ cpu_has_mips_r6)

-#define cpu_has_mips32 (cpu_has_mips32r1 | cpu_has_mips32r2 | cpu_has_mips32r6)
+#define cpu_has_mips32 (cpu_has_mips32r1 | cpu_has_mips32r2 | \
+ cpu_has_mips32r5 | cpu_has_mips32r6)
#define cpu_has_mips64 (cpu_has_mips64r1 | cpu_has_mips64r2 | cpu_has_mips64r6)
#define cpu_has_mips_r1 (cpu_has_mips32r1 | cpu_has_mips64r1)
#define cpu_has_mips_r2 (cpu_has_mips32r2 | cpu_has_mips64r2)
+#define cpu_has_mips_r5 (cpu_has_mips32r5)
#define cpu_has_mips_r6 (cpu_has_mips32r6 | cpu_has_mips64r6)
#define cpu_has_mips_r (cpu_has_mips32r1 | cpu_has_mips32r2 | \
- cpu_has_mips32r6 | cpu_has_mips64r1 | \
- cpu_has_mips64r2 | cpu_has_mips64r6)
+ cpu_has_mips32r5 | cpu_has_mips32r6 | \
+ cpu_has_mips64r1 | cpu_has_mips64r2 | \
+ cpu_has_mips64r6)

-/* MIPSR2 and MIPSR6 have a lot of similarities */
-#define cpu_has_mips_r2_r6 (cpu_has_mips_r2 | cpu_has_mips_r6)
+/* MIPSR2 - MIPSR6 have a lot of similarities */
+#define cpu_has_mips_r2_r6 (cpu_has_mips_r2 | cpu_has_mips_r5 | \
+ cpu_has_mips_r6)

/*
* cpu_has_mips_r2_exec_hazard - return if IHB is required on current processor
diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index ed7ffe4e63a3..bce3ea7fff7c 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -142,7 +142,7 @@ struct proc_cpuinfo_notifier_args {
static inline unsigned int cpu_cluster(struct cpuinfo_mips *cpuinfo)
{
/* Optimisation for systems where multiple clusters aren't used */
- if (!IS_ENABLED(CONFIG_CPU_MIPSR6))
+ if (!IS_ENABLED(CONFIG_CPU_MIPSR5) && !IS_ENABLED(CONFIG_CPU_MIPSR6))
return 0;

return (cpuinfo->globalnumber & MIPS_GLOBALNUMBER_CLUSTER) >>
diff --git a/arch/mips/include/asm/cpu-type.h b/arch/mips/include/asm/cpu-type.h
index 49f0061a6051..11090bc20464 100644
--- a/arch/mips/include/asm/cpu-type.h
+++ b/arch/mips/include/asm/cpu-type.h
@@ -51,11 +51,15 @@ static inline int __pure __get_cpu_type(const int cpu_type)
case CPU_M14KEC:
case CPU_INTERAPTIV:
case CPU_PROAPTIV:
- case CPU_P5600:
+#endif
+
+#ifdef CONFIG_SYS_HAS_CPU_MIPS32_R5
case CPU_M5150:
+ case CPU_P5600:
#endif

#if defined(CONFIG_SYS_HAS_CPU_MIPS32_R2) || \
+ defined(CONFIG_SYS_HAS_CPU_MIPS32_R5) || \
defined(CONFIG_SYS_HAS_CPU_MIPS32_R6) || \
defined(CONFIG_SYS_HAS_CPU_MIPS64_R2) || \
defined(CONFIG_SYS_HAS_CPU_MIPS64_R6)
diff --git a/arch/mips/include/asm/cpu.h b/arch/mips/include/asm/cpu.h
index 216a22916740..9bae99b568c9 100644
--- a/arch/mips/include/asm/cpu.h
+++ b/arch/mips/include/asm/cpu.h
@@ -343,11 +343,12 @@ enum cpu_type_enum {
#define MIPS_CPU_ISA_M32R2 0x00000020
#define MIPS_CPU_ISA_M64R1 0x00000040
#define MIPS_CPU_ISA_M64R2 0x00000080
-#define MIPS_CPU_ISA_M32R6 0x00000100
-#define MIPS_CPU_ISA_M64R6 0x00000200
+#define MIPS_CPU_ISA_M32R5 0x00000100
+#define MIPS_CPU_ISA_M32R6 0x00000400
+#define MIPS_CPU_ISA_M64R6 0x00000800

#define MIPS_CPU_ISA_32BIT (MIPS_CPU_ISA_II | MIPS_CPU_ISA_M32R1 | \
- MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M32R6)
+ MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M32R5 | MIPS_CPU_ISA_M32R6)
#define MIPS_CPU_ISA_64BIT (MIPS_CPU_ISA_III | MIPS_CPU_ISA_IV | \
MIPS_CPU_ISA_V | MIPS_CPU_ISA_M64R1 | MIPS_CPU_ISA_M64R2 | \
MIPS_CPU_ISA_M64R6)
diff --git a/arch/mips/include/asm/fpu.h b/arch/mips/include/asm/fpu.h
index 9476e0498d59..f0b37663fade 100644
--- a/arch/mips/include/asm/fpu.h
+++ b/arch/mips/include/asm/fpu.h
@@ -71,8 +71,8 @@ static inline int __enable_fpu(enum fpu_mode mode)
goto fr_common;

case FPU_64BIT:
-#if !(defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6) \
- || defined(CONFIG_64BIT))
+#if !(defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_MIPSR6) || defined(CONFIG_64BIT))
/* we only have a 32-bit FPU */
return SIGFPE;
#endif
diff --git a/arch/mips/include/asm/hazards.h b/arch/mips/include/asm/hazards.h
index a0b92205f933..f855478d12fa 100644
--- a/arch/mips/include/asm/hazards.h
+++ b/arch/mips/include/asm/hazards.h
@@ -22,8 +22,9 @@
/*
* TLB hazards
*/
-#if (defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)) && \
- !defined(CONFIG_CPU_CAVIUM_OCTEON) && !defined(CONFIG_CPU_LOONGSON64)
+#if (defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_MIPSR6)) && \
+ !defined(CONFIG_CPU_CAVIUM_OCTEON) && !defined(CONFIG_CPU_LOONGSON64)

/*
* MIPSR2 defines ehb for hazard avoidance
@@ -278,7 +279,8 @@ do { \

#define __disable_fpu_hazard

-#elif defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6)
+#elif defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_MIPSR6)

#define __enable_fpu_hazard \
___ehb
diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
index 4d6ad907ae54..3e8d2aaf96af 100644
--- a/arch/mips/include/asm/stackframe.h
+++ b/arch/mips/include/asm/stackframe.h
@@ -424,7 +424,7 @@

.macro RESTORE_SP_AND_RET docfi=0
RESTORE_SP \docfi
-#ifdef CONFIG_CPU_MIPSR6
+#if defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
eretnc
#else
.set push
diff --git a/arch/mips/include/asm/switch_to.h b/arch/mips/include/asm/switch_to.h
index 09cbe9042828..0b0a93bf83cd 100644
--- a/arch/mips/include/asm/switch_to.h
+++ b/arch/mips/include/asm/switch_to.h
@@ -67,11 +67,11 @@ do { \
#endif

/*
- * Clear LLBit during context switches on MIPSr6 such that eretnc can be used
+ * Clear LLBit during context switches on MIPSr5+ such that eretnc can be used
* unconditionally when returning to userland in entry.S.
*/
-#define __clear_r6_hw_ll_bit() do { \
- if (cpu_has_mips_r6) \
+#define __clear_r5_hw_ll_bit() do { \
+ if (cpu_has_mips_r5 || cpu_has_mips_r6) \
write_c0_lladdr(0); \
} while (0)

@@ -129,7 +129,7 @@ do { \
} \
clear_c0_status(ST0_CU2); \
} \
- __clear_r6_hw_ll_bit(); \
+ __clear_r5_hw_ll_bit(); \
__clear_software_ll_bit(); \
if (cpu_has_userlocal) \
write_c0_userlocal(task_thread_info(next)->tp_value); \
diff --git a/arch/mips/include/asm/vermagic.h b/arch/mips/include/asm/vermagic.h
index 24dc3d35161c..5a0e739f597a 100644
--- a/arch/mips/include/asm/vermagic.h
+++ b/arch/mips/include/asm/vermagic.h
@@ -8,6 +8,8 @@
#define MODULE_PROC_FAMILY "MIPS32_R1 "
#elif defined CONFIG_CPU_MIPS32_R2
#define MODULE_PROC_FAMILY "MIPS32_R2 "
+#elif defined CONFIG_CPU_MIPS32_R5
+#define MODULE_PROC_FAMILY "MIPS32_R5 "
#elif defined CONFIG_CPU_MIPS32_R6
#define MODULE_PROC_FAMILY "MIPS32_R6 "
#elif defined CONFIG_CPU_MIPS64_R1
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index f21a2304401f..a2dafef2df45 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -92,6 +92,7 @@ static void cpu_set_fpu_2008(struct cpuinfo_mips *c)
{
if (c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6)) {
unsigned long sr, fir, fcsr, fcsr0, fcsr1;

@@ -172,6 +173,7 @@ static void cpu_set_nofpu_2008(struct cpuinfo_mips *c)
case STRICT:
if (c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6)) {
c->options |= MIPS_CPU_NAN_2008 | MIPS_CPU_NAN_LEGACY;
} else {
@@ -263,9 +265,11 @@ static void cpu_set_nofpu_id(struct cpuinfo_mips *c)
value = 0;
if (c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6))
value |= MIPS_FPIR_D | MIPS_FPIR_S;
if (c->isa_level & (MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6))
value |= MIPS_FPIR_F64 | MIPS_FPIR_L | MIPS_FPIR_W;
if (c->options & MIPS_CPU_NAN_2008)
@@ -286,6 +290,7 @@ static void cpu_set_fpu_opts(struct cpuinfo_mips *c)

if (c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6)) {
if (c->fpu_id & MIPS_FPIR_3D)
c->ases |= MIPS_ASE_MIPS3D;
@@ -563,6 +568,9 @@ static void set_isa(struct cpuinfo_mips *c, unsigned int isa)
set_elf_base_platform("mips32r6");
/* Break here so we don't add incompatible ISAs */
break;
+ case MIPS_CPU_ISA_M32R5:
+ c->isa_level |= MIPS_CPU_ISA_M32R5;
+ /* fall through */
case MIPS_CPU_ISA_M32R2:
c->isa_level |= MIPS_CPU_ISA_M32R2;
set_elf_base_platform("mips32r2");
@@ -1751,6 +1759,10 @@ static inline void cpu_probe_mips(struct cpuinfo_mips *c, unsigned int cpu)
spram_config();

switch (__get_cpu_type(c->cputype)) {
+ case CPU_M5150:
+ case CPU_P5600:
+ set_isa(c, MIPS_CPU_ISA_M32R5);
+ break;
case CPU_I6500:
c->options |= MIPS_CPU_SHARED_FTLB_ENTRIES;
/* fall-through */
diff --git a/arch/mips/kernel/entry.S b/arch/mips/kernel/entry.S
index 4849a48afc0f..4b896f5023ff 100644
--- a/arch/mips/kernel/entry.S
+++ b/arch/mips/kernel/entry.S
@@ -169,8 +169,8 @@ syscall_exit_work:
jal syscall_trace_leave
b resume_userspace

-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR6) || \
- defined(CONFIG_MIPS_MT)
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_MIPSR6) || defined(CONFIG_MIPS_MT)

/*
* MIPS32R2 Instruction Hazard Barrier - must be called
@@ -183,4 +183,4 @@ LEAF(mips_ihb)
nop
END(mips_ihb)

-#endif /* CONFIG_CPU_MIPSR2 or CONFIG_CPU_MIPSR6 or CONFIG_MIPS_MT */
+#endif /* CONFIG_CPU_MIPSR2 - CONFIG_CPU_MIPSR6 or CONFIG_MIPS_MT */
diff --git a/arch/mips/kernel/proc.c b/arch/mips/kernel/proc.c
index f8d36710cd58..b0ffb968f07d 100644
--- a/arch/mips/kernel/proc.c
+++ b/arch/mips/kernel/proc.c
@@ -98,6 +98,8 @@ static int show_cpuinfo(struct seq_file *m, void *v)
seq_printf(m, "%s", " mips32r1");
if (cpu_has_mips32r2)
seq_printf(m, "%s", " mips32r2");
+ if (cpu_has_mips32r5)
+ seq_printf(m, "%s", " mips32r5");
if (cpu_has_mips32r6)
seq_printf(m, "%s", " mips32r6");
if (cpu_has_mips64r1)
diff --git a/arch/mips/kernel/r4k_fpu.S b/arch/mips/kernel/r4k_fpu.S
index 59be5c812aa2..b91e91106475 100644
--- a/arch/mips/kernel/r4k_fpu.S
+++ b/arch/mips/kernel/r4k_fpu.S
@@ -41,7 +41,7 @@
LEAF(_save_fp)
EXPORT_SYMBOL(_save_fp)
#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
- defined(CONFIG_CPU_MIPSR6)
+ defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
mfc0 t0, CP0_STATUS
#endif
fpu_save_double a0 t0 t1 # clobbers t1
@@ -53,7 +53,7 @@ EXPORT_SYMBOL(_save_fp)
*/
LEAF(_restore_fp)
#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
- defined(CONFIG_CPU_MIPSR6)
+ defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
mfc0 t0, CP0_STATUS
#endif
fpu_restore_double a0 t0 t1 # clobbers t1
@@ -103,10 +103,10 @@ LEAF(_save_fp_context)
.set pop

#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
- defined(CONFIG_CPU_MIPSR6)
+ defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
.set push
SET_HARDFLOAT
-#ifdef CONFIG_CPU_MIPSR2
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5)
.set mips32r2
.set fp=64
mfc0 t0, CP0_STATUS
@@ -170,11 +170,11 @@ LEAF(_save_fp_context)
LEAF(_restore_fp_context)
EX lw t1, 0(a1)

-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
- defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
+ defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
.set push
SET_HARDFLOAT
-#ifdef CONFIG_CPU_MIPSR2
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5)
.set mips32r2
.set fp=64
mfc0 t0, CP0_STATUS
diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
index dde20887a70d..66432b5ab229 100644
--- a/arch/mips/kvm/vz.c
+++ b/arch/mips/kvm/vz.c
@@ -2980,7 +2980,7 @@ static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
*/

/* PageGrain */
- if (cpu_has_mips_r6)
+ if (cpu_has_mips_r5 || cpu_has_mips_r6)
kvm_write_sw_gc0_pagegrain(cop0, PG_RIE | PG_XIE | PG_IEC);
/* Wired */
if (cpu_has_mips_r6)
@@ -2988,7 +2988,7 @@ static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
read_gc0_wired() & MIPSR6_WIRED_LIMIT);
/* Status */
kvm_write_sw_gc0_status(cop0, ST0_BEV | ST0_ERL);
- if (cpu_has_mips_r6)
+ if (cpu_has_mips_r5 || cpu_has_mips_r6)
kvm_change_sw_gc0_status(cop0, ST0_FR, read_gc0_status());
/* IntCtl */
kvm_write_sw_gc0_intctl(cop0, read_gc0_intctl() &
@@ -3086,7 +3086,7 @@ static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
}

/* reset HTW registers */
- if (cpu_guest_has_htw && cpu_has_mips_r6) {
+ if (cpu_guest_has_htw && (cpu_has_mips_r5 || cpu_has_mips_r6)) {
/* PWField */
kvm_write_sw_gc0_pwfield(cop0, 0x0c30c302);
/* PWSize */
diff --git a/arch/mips/lib/csum_partial.S b/arch/mips/lib/csum_partial.S
index fda7b57b826e..87fda0713b84 100644
--- a/arch/mips/lib/csum_partial.S
+++ b/arch/mips/lib/csum_partial.S
@@ -279,7 +279,8 @@ EXPORT_SYMBOL(csum_partial)
#endif

/* odd buffer alignment? */
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_LOONGSON64)
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_LOONGSON64)
.set push
.set arch=mips32r2
wsbh v1, sum
@@ -732,7 +733,8 @@ EXPORT_SYMBOL(csum_partial)
addu sum, v1
#endif

-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_LOONGSON64)
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
+ defined(CONFIG_CPU_LOONGSON64)
.set push
.set arch=mips32r2
wsbh v1, sum
diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 36a311348739..99146abfbbca 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -1703,9 +1703,10 @@ static void setup_scache(void)
return;

default:
- if (c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M32R2 |
- MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R1 |
- MIPS_CPU_ISA_M64R2 | MIPS_CPU_ISA_M64R6)) {
+ if (c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
+ MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
+ MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6)) {
#ifdef CONFIG_MIPS_CPU_SCACHE
if (mips_sc_init ()) {
scache_size = c->scache.ways * c->scache.sets * c->scache.linesz;
diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
index dbdbfe5d8408..6fae3014ad3e 100644
--- a/arch/mips/mm/sc-mips.c
+++ b/arch/mips/mm/sc-mips.c
@@ -194,9 +194,10 @@ static inline int __init mips_sc_probe(void)
return mips_sc_probe_cm3();

/* Ignore anything but MIPSxx processors */
- if (!(c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M32R2 |
- MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R1 |
- MIPS_CPU_ISA_M64R2 | MIPS_CPU_ISA_M64R6)))
+ if (!(c->isa_level & (MIPS_CPU_ISA_M32R1 | MIPS_CPU_ISA_M64R1 |
+ MIPS_CPU_ISA_M32R2 | MIPS_CPU_ISA_M64R2 |
+ MIPS_CPU_ISA_M32R5 |
+ MIPS_CPU_ISA_M32R6 | MIPS_CPU_ISA_M64R6)))
return 0;

/* Does this MIPS32/MIPS64 CPU have a config2 register? */
--
2.25.1

2020-05-06 17:48:54

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors

From: Serge Semin <[email protected]>

Some platforms may prohibit to access the IO-memory with instructions
of certain memory widths. For instance Bailal-T1 has devices placed
behind memory OCP port (which also the reason of DMA accesses being
incoherent) and can't be accessed through CCA uncacheable memory with
other than 4-bytes aligned (LW/SW) instructions. Ignoring this rule
will cause the APB EHB error with 0xFFs returned on read operations.
In order to fix the issue for this platform and for others, which may
have similar problems, lets recode serial_in()/serial_out() to call
a certain memory accessors in accordance with the UART registers shift
setting.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/early_printk_8250.c | 34 ++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/early_printk_8250.c b/arch/mips/kernel/early_printk_8250.c
index 567c6ec0cfae..e2c2405cff62 100644
--- a/arch/mips/kernel/early_printk_8250.c
+++ b/arch/mips/kernel/early_printk_8250.c
@@ -23,12 +23,42 @@ void setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,

static inline u8 serial_in(int offset)
{
- return readb(serial8250_base + (offset << serial8250_reg_shift));
+ u8 ret = 0xFF;
+
+ offset <<= serial8250_reg_shift;
+ switch (serial8250_reg_shift) {
+ case 0:
+ ret = readb(serial8250_base + offset);
+ break;
+ case 1:
+ ret = readw(serial8250_base + offset);
+ break;
+ case 2:
+ ret = readl(serial8250_base + offset);
+ break;
+ default:
+ break;
+ }
+
+ return ret;
}

static inline void serial_out(int offset, char value)
{
- writeb(value, serial8250_base + (offset << serial8250_reg_shift));
+ offset <<= serial8250_reg_shift;
+ switch (serial8250_reg_shift) {
+ case 0:
+ writeb(value, serial8250_base + offset);
+ break;
+ case 1:
+ writew(value, serial8250_base + offset);
+ break;
+ case 2:
+ writel(value, serial8250_base + offset);
+ break;
+ default:
+ break;
+ }
}

void prom_putchar(char c)
--
2.25.1

2020-05-06 17:49:27

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

From: Serge Semin <[email protected]>

Commit 07d69579e7fe ("MIPS: Don't register r4k sched clock when
CPUFREQ enabled") disabled the r4k-clock usage for scheduler ticks
counting due to the scheduler being non-tolerant for unstable
clocks sources. For the same reason the clock should be used
in the system clocksource framework only as a last resort if CPU
frequency may change.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/csrc-r4k.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
index 437dda64fd7a..d81fb374f477 100644
--- a/arch/mips/kernel/csrc-r4k.c
+++ b/arch/mips/kernel/csrc-r4k.c
@@ -71,7 +71,11 @@ int __init init_r4k_clocksource(void)
return -ENXIO;

/* Calculate a somewhat reasonable rating value */
+#ifndef CONFIG_CPU_FREQ
clocksource_mips.rating = 200 + mips_hpt_frequency / 10000000;
+#else
+ clocksource_mips.rating = 99;
+#endif

/*
* R2 onwards makes the count accessible to user mode so it can be used
--
2.25.1

2020-05-06 17:49:39

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 12/20] mips: MAAR: Add XPA mode support

From: Serge Semin <[email protected]>

When XPA mode is enabled the normally 32-bits MAAR pair registers
are extended to be of 64-bits width as in pure 64-bits MIPS
architecture. In this case the MAAR registers can enable the
speculative loads/stores for addresses of up to 39-bits width.
But in this case the process of the MAAR initialization changes a bit.
The upper 32-bits of the registers are supposed to be accessed by mean
of the dedicated instructions mfhc0/mthc0 and there is a CP0.MAAR.VH
bit which should be set together with CP0.MAAR.VL as indication
of the boundary validity. All of these peculiarities were taken into
account in this commit so the speculative loads/stores would work
when XPA mode is enabled.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/maar.h | 17 +++++++++++++++--
arch/mips/include/asm/mipsregs.h | 10 ++++++++++
arch/mips/mm/init.c | 8 +++++++-
3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
index 6908b93c4ff9..99f1c3e4b11f 100644
--- a/arch/mips/include/asm/maar.h
+++ b/arch/mips/include/asm/maar.h
@@ -32,7 +32,7 @@ unsigned platform_maar_init(unsigned num_pairs);
* @upper: The highest address that the MAAR pair will affect. Must be
* aligned to one byte before a 2^16 byte boundary.
* @attrs: The accessibility attributes to program, eg. MIPS_MAAR_S. The
- * MIPS_MAAR_VL attribute will automatically be set.
+ * MIPS_MAAR_VL/MIPS_MAAR_VH attributes will automatically be set.
*
* Program the pair of MAAR registers specified by idx to apply the attributes
* specified by attrs to the range of addresses from lower to higher.
@@ -48,17 +48,30 @@ static inline void write_maar_pair(unsigned idx, phys_addr_t lower,
/* Automatically set MIPS_MAAR_VL */
attrs |= MIPS_MAAR_VL;

- /* Write the upper address & attributes (only MIPS_MAAR_VL matters) */
+ /*
+ * Write the upper address & attributes (both MIPS_MAAR_VL and
+ * MIPS_MAAR_VH matter)
+ */
write_c0_maari(idx << 1);
back_to_back_c0_hazard();
write_c0_maar(((upper >> 4) & MIPS_MAAR_ADDR) | attrs);
back_to_back_c0_hazard();
+#ifdef CONFIG_XPA
+ upper >>= MIPS_MAARX_ADDR_SHIFT;
+ writex_c0_maar(((upper >> 4) & MIPS_MAARX_ADDR) | MIPS_MAARX_VH);
+ back_to_back_c0_hazard();
+#endif

/* Write the lower address & attributes */
write_c0_maari((idx << 1) | 0x1);
back_to_back_c0_hazard();
write_c0_maar((lower >> 4) | attrs);
back_to_back_c0_hazard();
+#ifdef CONFIG_XPA
+ lower >>= MIPS_MAARX_ADDR_SHIFT;
+ writex_c0_maar(((lower >> 4) & MIPS_MAARX_ADDR) | MIPS_MAARX_VH);
+ back_to_back_c0_hazard();
+#endif
}

/**
diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 165f6318d861..aa8599962ea3 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -779,6 +779,14 @@
#define MIPS_MAAR_ADDR_SHIFT 12
#define MIPS_MAAR_S (_ULCAST_(1) << 1)
#define MIPS_MAAR_VL (_ULCAST_(1) << 0)
+#ifdef CONFIG_XPA
+#define MIPS_MAAR_V (MIPS_MAAR_VH | MIPS_MAAR_VL)
+#else
+#define MIPS_MAAR_V MIPS_MAAR_VL
+#endif
+#define MIPS_MAARX_VH (_ULCAST_(1) << 31)
+#define MIPS_MAARX_ADDR 0xF
+#define MIPS_MAARX_ADDR_SHIFT 32

/* MAARI bit definitions */
#define MIPS_MAARI_INDEX (_ULCAST_(0x3f) << 0)
@@ -1739,6 +1747,8 @@ do { \
#define write_c0_lladdr(val) __write_ulong_c0_register($17, 0, val)
#define read_c0_maar() __read_ulong_c0_register($17, 1)
#define write_c0_maar(val) __write_ulong_c0_register($17, 1, val)
+#define readx_c0_maar() __readx_32bit_c0_register($17, 1)
+#define writex_c0_maar(val) __writex_32bit_c0_register($17, 1, val)
#define read_c0_maari() __read_32bit_c0_register($17, 2)
#define write_c0_maari(val) __write_32bit_c0_register($17, 2, val)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 79684000de0e..620ebfa45ec1 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -358,17 +358,23 @@ void maar_init(void)
write_c0_maari(i);
back_to_back_c0_hazard();
upper = read_c0_maar();
+#ifdef CONFIG_XPA
+ upper |= (phys_addr_t)readx_c0_maar() << MIPS_MAARX_ADDR_SHIFT;
+#endif

write_c0_maari(i + 1);
back_to_back_c0_hazard();
lower = read_c0_maar();
+#ifdef CONFIG_XPA
+ lower |= (phys_addr_t)readx_c0_maar() << MIPS_MAARX_ADDR_SHIFT;
+#endif

attr = lower & upper;
lower = (lower & MIPS_MAAR_ADDR) << 4;
upper = ((upper & MIPS_MAAR_ADDR) << 4) | 0xffff;

pr_info(" [%d]: ", i / 2);
- if (!(attr & MIPS_MAAR_VL)) {
+ if ((attr & MIPS_MAAR_V) != MIPS_MAAR_V) {
pr_cont("disabled\n");
continue;
}
--
2.25.1

2020-05-06 17:58:31

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

Hi Sergey.

On Wed, May 06, 2020 at 08:42:21PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> Add "BAIKAL ELECTRONICS, JSC" to the list of devicetree vendor prefixes
> as "baikal".
>
> Website: http://www.baikalelectronics.com
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---
>
> Changelog v2:
> - Fix author and SoB emails mismatch.

> - Add 'baikal' vendor prefix instead of ambiguous 'be'.
Agree, much better.

> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index d3891386d671..674c0d07c0ad 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -139,6 +139,8 @@ patternProperties:
> description: Azoteq (Pty) Ltd
> "^azw,.*":
> description: Shenzhen AZW Technology Co., Ltd.
> + "^baikal,.*":
> + description: BAIKAL ELECTRONICS, JSC
Baikal do not use ALL UPPSECASE on their website for their name.
So please use same case use as they do themself.


Sam

> "^bananapi,.*":
> description: BIPAI KEJI LIMITED
> "^beacon,.*":
> --
> 2.25.1

2020-05-06 19:52:07

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 05/20] mips: cm: Add L2 ECC/parity errors reporting

From: Serge Semin <[email protected]>

According to the MIPS32 InterAptiv software manual error codes 24 - 26
of CM2 indicate L2 ECC/parity error with switching to a corresponding
errors info fields. This patch provides these errors parsing code,
which handles the read/write uncorrectable and correctable ECC/parity
errors, and prints instruction causing the fault, RAM array type, cache
way/dword and syndrome associated with the faulty data.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/mips-cm.c | 62 ++++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index 361bfc91a0e6..f60af512c877 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -114,6 +114,48 @@ static char *cm2_core[8] = {
"Exclusive/OK", "Exclusive/Data"
};

+static char *cm2_l2_type[4] = {
+ [0x0] = "None",
+ [0x1] = "Tag RAM single/double ECC error",
+ [0x2] = "Data RAM single/double ECC error",
+ [0x3] = "WS RAM uncorrectable dirty parity"
+};
+
+static char *cm2_l2_instr[32] = {
+ [0x00] = "L2_NOP",
+ [0x01] = "L2_ERR_CORR",
+ [0x02] = "L2_TAG_INV",
+ [0x03] = "L2_WS_CLEAN",
+ [0x04] = "L2_RD_MDYFY_WR",
+ [0x05] = "L2_WS_MRU",
+ [0x06] = "L2_EVICT_LN2",
+ [0x07] = "0x07",
+ [0x08] = "L2_EVICT",
+ [0x09] = "L2_REFL",
+ [0x0a] = "L2_RD",
+ [0x0b] = "L2_WR",
+ [0x0c] = "L2_EVICT_MRU",
+ [0x0d] = "L2_SYNC",
+ [0x0e] = "L2_REFL_ERR",
+ [0x0f] = "0x0f",
+ [0x10] = "L2_INDX_WB_INV",
+ [0x11] = "L2_INDX_LD_TAG",
+ [0x12] = "L2_INDX_ST_TAG",
+ [0x13] = "L2_INDX_ST_DATA",
+ [0x14] = "L2_INDX_ST_ECC",
+ [0x15] = "0x15",
+ [0x16] = "0x16",
+ [0x17] = "0x17",
+ [0x18] = "L2_FTCH_AND_LCK",
+ [0x19] = "L2_HIT_INV",
+ [0x1a] = "L2_HIT_WB_INV",
+ [0x1b] = "L2_HIT_WB",
+ [0x1c] = "0x1c",
+ [0x1d] = "0x1d",
+ [0x1e] = "0x1e",
+ [0x1f] = "0x1f"
+};
+
static char *cm2_causes[32] = {
"None", "GC_WR_ERR", "GC_RD_ERR", "COH_WR_ERR",
"COH_RD_ERR", "MMIO_WR_ERR", "MMIO_RD_ERR", "0x07",
@@ -121,7 +163,7 @@ static char *cm2_causes[32] = {
"0x0c", "0x0d", "0x0e", "0x0f",
"0x10", "INTVN_WR_ERR", "INTVN_RD_ERR", "0x13",
"0x14", "0x15", "0x16", "0x17",
- "0x18", "0x19", "0x1a", "0x1b",
+ "L2_RD_UNCORR", "L2_WR_UNCORR", "L2_CORR", "0x1b",
"0x1c", "0x1d", "0x1e", "0x1f"
};

@@ -360,7 +402,7 @@ void mips_cm_error_report(void)
"CCA=%lu TR=%s MCmd=%s STag=%lu "
"SPort=%lu\n", cca_bits, cm2_tr[tr_bits],
cm2_cmd[cmd_bits], stag_bits, sport_bits);
- } else {
+ } else if (cause < 24) {
/* glob state & sresp together */
unsigned long c3_bits = (cm_error >> 18) & 7;
unsigned long c2_bits = (cm_error >> 15) & 7;
@@ -377,6 +419,22 @@ void mips_cm_error_report(void)
cm2_core[c1_bits], cm2_core[c0_bits],
sc_bit ? "True" : "False",
cm2_cmd[cmd_bits], sport_bits);
+ } else {
+ unsigned long muc_bit = (cm_error >> 23) & 1;
+ unsigned long ins_bits = (cm_error >> 18) & 0x1f;
+ unsigned long arr_bits = (cm_error >> 16) & 3;
+ unsigned long dw_bits = (cm_error >> 12) & 15;
+ unsigned long way_bits = (cm_error >> 9) & 7;
+ unsigned long mway_bit = (cm_error >> 8) & 1;
+ unsigned long syn_bits = (cm_error >> 0) & 0xFF;
+
+ snprintf(buf, sizeof(buf),
+ "Type=%s%s Instr=%s DW=%lu Way=%lu "
+ "MWay=%s Syndrome=0x%02lx",
+ muc_bit ? "Multi-UC " : "",
+ cm2_l2_type[arr_bits],
+ cm2_l2_instr[ins_bits], dw_bits, way_bits,
+ mway_bit ? "True" : "False", syn_bits);
}
pr_err("CM_ERROR=%08llx %s <%s>\n", cm_error,
cm2_causes[cause], buf);
--
2.25.1

2020-05-06 19:53:09

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 14/20] mips: Use offset-sized IO-mem accessors in CPS debug printout

From: Serge Semin <[email protected]>

Similar to commit 8e5c62e38a88 ("mips: early_printk_8250: Use offset-sized
IO-mem accessors") the IO-memory might require to use a proper load/store
instructions (like Bailal-T1 IO-memory). To fix the cps-vec UART debug
printout lets use the memory access instructions in accordance with the
UART registers offset config specified at boot time.

Signed-off-by: Serge Semin <[email protected]>
Cc: Alexey Malahov <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
There might be another problem in cps-vec-ns16550.S connected with the
difference in CPU/devices endinanness on some platforms. But there is
no such for Baikal-T1 SoC.
---
arch/mips/kernel/cps-vec-ns16550.S | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/cps-vec-ns16550.S b/arch/mips/kernel/cps-vec-ns16550.S
index d5a67b4ce9f6..2adb1c56c7c5 100644
--- a/arch/mips/kernel/cps-vec-ns16550.S
+++ b/arch/mips/kernel/cps-vec-ns16550.S
@@ -14,16 +14,30 @@
#define UART_TX_OFS (UART_TX << CONFIG_MIPS_CPS_NS16550_SHIFT)
#define UART_LSR_OFS (UART_LSR << CONFIG_MIPS_CPS_NS16550_SHIFT)

+#if CONFIG_MIPS_CPS_NS16550_SHIFT == 0
+# define UART_L lb
+# define UART_S sb
+#elif CONFIG_MIPS_CPS_NS16550_SHIFT == 1
+# define UART_L lh
+# define UART_S sh
+#elif CONFIG_MIPS_CPS_NS16550_SHIFT == 2
+# define UART_L lw
+# define UART_S sw
+#else
+# define UART_L lw
+# define UART_S sb
+#endif
+
/**
* _mips_cps_putc() - write a character to the UART
* @a0: ASCII character to write
* @t9: UART base address
*/
LEAF(_mips_cps_putc)
-1: lw t0, UART_LSR_OFS(t9)
+1: UART_L t0, UART_LSR_OFS(t9)
andi t0, t0, UART_LSR_TEMT
beqz t0, 1b
- sb a0, UART_TX_OFS(t9)
+ UART_S a0, UART_TX_OFS(t9)
jr ra
END(_mips_cps_putc)

--
2.25.1

2020-05-06 20:05:40

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

Hi Sam,

On Wed, May 06, 2020 at 07:55:53PM +0200, Sam Ravnborg wrote:
> Hi Sergey.
>
> On Wed, May 06, 2020 at 08:42:21PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > Add "BAIKAL ELECTRONICS, JSC" to the list of devicetree vendor prefixes
> > as "baikal".
> >
> > Website: http://www.baikalelectronics.com
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > ---
> >
> > Changelog v2:
> > - Fix author and SoB emails mismatch.
>
> > - Add 'baikal' vendor prefix instead of ambiguous 'be'.
> Agree, much better.
>
> > ---
> > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index d3891386d671..674c0d07c0ad 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -139,6 +139,8 @@ patternProperties:
> > description: Azoteq (Pty) Ltd
> > "^azw,.*":
> > description: Shenzhen AZW Technology Co., Ltd.
> > + "^baikal,.*":
> > + description: BAIKAL ELECTRONICS, JSC
> Baikal do not use ALL UPPSECASE on their website for their name.
> So please use same case use as they do themself.
>

It's not like me can't be considered as part of them.) I discussed the
upper-case and normal version with our managers half a year ago and we
came up to use the upper-case version. From Russian legal point of view
it's also the upper-cased version what counts. I don't really know why
the site use different naming, but in the internal documents it's always
as submitted. Anyway I asked this question one more time to our managers.
If they say to use as you suggest, then I'll resend an update in v3
patchset, if v3 doesn't get to be necessary I'll send a followup patch
with fix.

-Sergey

>
> Sam
>
> > "^bananapi,.*":
> > description: BIPAI KEJI LIMITED
> > "^beacon,.*":
> > --
> > 2.25.1

2020-05-06 20:11:11

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

Hi Sergey
> > > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > > @@ -139,6 +139,8 @@ patternProperties:
> > > description: Azoteq (Pty) Ltd
> > > "^azw,.*":
> > > description: Shenzhen AZW Technology Co., Ltd.
> > > + "^baikal,.*":
> > > + description: BAIKAL ELECTRONICS, JSC
> > Baikal do not use ALL UPPSECASE on their website for their name.
> > So please use same case use as they do themself.
> >
>
> It's not like me can't be considered as part of them.) I discussed the
> upper-case and normal version with our managers half a year ago and we
> came up to use the upper-case version. From Russian legal point of view
> it's also the upper-cased version what counts. I don't really know why
> the site use different naming, but in the internal documents it's always
> as submitted. Anyway I asked this question one more time to our managers.
> If they say to use as you suggest, then I'll resend an update in v3
> patchset, if v3 doesn't get to be necessary I'll send a followup patch
> with fix.

I had expected it was upper case because others used upper case.
But there is a good explanation and then all is fine wiht me.

And it is an alphabetic order - so
Acked-by: Sam Ravnborg <[email protected]>

>
> -Sergey
>
> >
> > Sam
> >
> > > "^bananapi,.*":
> > > description: BIPAI KEJI LIMITED
> > > "^beacon,.*":
> > > --
> > > 2.25.1

2020-05-06 23:51:57

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

On Wed, May 06, 2020 at 09:26:53PM +0200, Sam Ravnborg wrote:
> Hi Sergey
> > > > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > > > @@ -139,6 +139,8 @@ patternProperties:
> > > > description: Azoteq (Pty) Ltd
> > > > "^azw,.*":
> > > > description: Shenzhen AZW Technology Co., Ltd.
> > > > + "^baikal,.*":
> > > > + description: BAIKAL ELECTRONICS, JSC
> > > Baikal do not use ALL UPPSECASE on their website for their name.
> > > So please use same case use as they do themself.
> > >
> >
> > It's not like me can't be considered as part of them.) I discussed the
> > upper-case and normal version with our managers half a year ago and we
> > came up to use the upper-case version. From Russian legal point of view
> > it's also the upper-cased version what counts. I don't really know why
> > the site use different naming, but in the internal documents it's always
> > as submitted. Anyway I asked this question one more time to our managers.
> > If they say to use as you suggest, then I'll resend an update in v3
> > patchset, if v3 doesn't get to be necessary I'll send a followup patch
> > with fix.
>
> I had expected it was upper case because others used upper case.
> But there is a good explanation and then all is fine wiht me.
>
> And it is an alphabetic order - so
> Acked-by: Sam Ravnborg <[email protected]>

Great. Thanks. I've just got an answer from our legal department:
"The Company has a Charter in which the name in English is:
“BAIKAL ELECTRONICS, JSC.” Charter is the only thing you need to focus on
when specifying the name."

I also pointed out that it would be good to have the site updated if the
upper-cased name is required by the Charter. Hope it will be done soon.)

-Sergey

>
> >
> > -Sergey
> >
> > >
> > > Sam
> > >
> > > > "^bananapi,.*":
> > > > description: BIPAI KEJI LIMITED
> > > > "^beacon,.*":
> > > > --
> > > > 2.25.1

2020-05-07 01:33:59

by Serge Semin

[permalink] [raw]
Subject: [PATCH v2 11/20] mips: MAAR: Use more precise address mask

From: Serge Semin <[email protected]>

Indeed according to the P5600/P6000 manual the MAAR pair register
address field either takes [12:31] bits for 32-bits non-XPA systems
and [12:35] otherwise. In any case the current address mask is just
wrong for 64-bit and 32-bits XPA chips. So lets extend it to 39-bits
value. This shall cover the 64-bits architecture and systems with XPA
enabled, and won't cause any problem for non-XPA 32-bit systems, since
the value will be just truncated when written to the 32-bits register.

Co-developed-by: Alexey Malahov <[email protected]>
Signed-off-by: Alexey Malahov <[email protected]>
Signed-off-by: Serge Semin <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/mipsregs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index 039ebd913f00..165f6318d861 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -775,7 +775,7 @@

/* MAAR bit definitions */
#define MIPS_MAAR_VH (_U64CAST_(1) << 63)
-#define MIPS_MAAR_ADDR ((BIT_ULL(BITS_PER_LONG - 12) - 1) << 12)
+#define MIPS_MAAR_ADDR GENMASK_ULL(35, 12)
#define MIPS_MAAR_ADDR_SHIFT 12
#define MIPS_MAAR_S (_ULCAST_(1) << 1)
#define MIPS_MAAR_VL (_ULCAST_(1) << 0)
--
2.25.1

2020-05-07 11:45:16

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 04/20] mips: cm: Fix an invalid error code of INTVN_*_ERR

On Wed, May 06, 2020 at 08:42:22PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> Commit 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache
> errors") adds cm2_causes[] array with map of error type ID and
> pointers to the short description string. There is a mistake in
> the table, since according to MIPS32 manual CM2_ERROR_TYPE = {17,18}
> correspond to INTVN_WR_ERR and INTVN_RD_ERR, while the table
> claims they have {0x17,0x18} codes. This is obviously hex-dec
> copy-paste bug. Moreover codes {0x18 - 0x1a} indicate L2 ECC errors.
>
> Fixes: 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache errors")
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/mips/kernel/mips-cm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

applied to mips-next.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-07 11:45:21

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 05/20] mips: cm: Add L2 ECC/parity errors reporting

On Wed, May 06, 2020 at 08:42:23PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> According to the MIPS32 InterAptiv software manual error codes 24 - 26
> of CM2 indicate L2 ECC/parity error with switching to a corresponding
> errors info fields. This patch provides these errors parsing code,
> which handles the read/write uncorrectable and correctable ECC/parity
> errors, and prints instruction causing the fault, RAM array type, cache
> way/dword and syndrome associated with the faulty data.
>
> Co-developed-by: Alexey Malahov <[email protected]>
> Signed-off-by: Alexey Malahov <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/mips/kernel/mips-cm.c | 62 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 60 insertions(+), 2 deletions(-)

applied to mips-next.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-07 11:45:29

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 11/20] mips: MAAR: Use more precise address mask

On Wed, May 06, 2020 at 08:42:29PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> Indeed according to the P5600/P6000 manual the MAAR pair register
> address field either takes [12:31] bits for 32-bits non-XPA systems
> and [12:35] otherwise. In any case the current address mask is just
> wrong for 64-bit and 32-bits XPA chips. So lets extend it to 39-bits
> value. This shall cover the 64-bits architecture and systems with XPA
> enabled, and won't cause any problem for non-XPA 32-bit systems, since
> the value will be just truncated when written to the 32-bits register.

according to MIPS32 Priveleged Resoure Architecture Rev. 6.02
ADDR spans from bit 12 to bit 55. So your patch fits only for P5600.
Does the wider mask cause any problems ?

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-07 11:46:03

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 07/20] mips: Add MIPS Warrior P5600 support

On Wed, May 06, 2020 at 08:42:25PM +0300, [email protected] wrote:
>
> +config CPU_P5600
> + bool "MIPS Warrior P5600"
> + depends on SYS_HAS_CPU_P5600
> + select CPU_HAS_PREFETCH
> + select CPU_SUPPORTS_32BIT_KERNEL
> + select CPU_SUPPORTS_HIGHMEM
> + select CPU_SUPPORTS_MSA
> + select CPU_SUPPORTS_UNCACHED_ACCELERATED
> + select CPU_SUPPORTS_CPUFREQ
> + select CPU_MIPSR2_IRQ_VI
> + select CPU_MIPSR2_IRQ_EI
> + select HAVE_KVM
> + select MIPS_O32_FP64_SUPPORT
> + help
> + Choose this option to build a kernel for MIPS Warrior P5600 CPU.
> + It's based on MIPS32r5 ISA with XPA, EVA, dual/quad issue exec pipes,
> + MMU with two-levels TLB, UCA, MSA, MDU core level features and system
> + level features like up to six P5600 calculation cores, CM2 with L2
> + cache, IOCU/IOMMU (though might be unused depending on the system-
> + specific IP core configuration), GIC, CPC, virtualisation module,
> + eJTAG and PDtrace.
> +
> config CPU_R3000
> bool "R3000"
> depends on SYS_HAS_CPU_R3000
> @@ -1841,7 +1863,8 @@ endchoice
> config CPU_MIPS32_3_5_FEATURES
> bool "MIPS32 Release 3.5 Features"
> depends on SYS_HAS_CPU_MIPS32_R3_5
> - depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6
> + depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6 || \
> + CPU_P5600
> help
> Choose this option to build a kernel for release 2 or later of the
> MIPS32 architecture including features from the 3.5 release such as
> @@ -1861,7 +1884,7 @@ config CPU_MIPS32_3_5_EVA
> config CPU_MIPS32_R5_FEATURES
> bool "MIPS32 Release 5 Features"
> depends on SYS_HAS_CPU_MIPS32_R5
> - depends on CPU_MIPS32_R2 || CPU_MIPS32_R5
> + depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_P5600
> help
> Choose this option to build a kernel for release 2 or later of the
> MIPS32 architecture including features from release 5 such as
> @@ -2016,6 +2039,10 @@ config SYS_HAS_CPU_MIPS64_R6
> bool
> select ARCH_HAS_SYNC_DMA_FOR_CPU if DMA_NONCOHERENT
>
> +config SYS_HAS_CPU_P5600
> + bool
> + select ARCH_HAS_SYNC_DMA_FOR_CPU if DMA_NONCOHERENT
> +

P5600 is CPU_MIPS_R5 so can't you select it here and drop all the || CPU_5600
above/below ?

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-07 19:15:53

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 11/20] mips: MAAR: Use more precise address mask

On Thu, May 07, 2020 at 01:09:51PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:29PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > Indeed according to the P5600/P6000 manual the MAAR pair register
> > address field either takes [12:31] bits for 32-bits non-XPA systems
> > and [12:35] otherwise. In any case the current address mask is just
> > wrong for 64-bit and 32-bits XPA chips. So lets extend it to 39-bits
> > value. This shall cover the 64-bits architecture and systems with XPA
> > enabled, and won't cause any problem for non-XPA 32-bit systems, since
> > the value will be just truncated when written to the 32-bits register.
>
> according to MIPS32 Priveleged Resoure Architecture Rev. 6.02
> ADDR spans from bit 12 to bit 55. So your patch fits only for P5600.

> Does the wider mask cause any problems ?

No, it won't. Bits written to the [40:62] range will be just ignored,
while reading from there should return zeros. Setting GENMASK_ULL(55, 12)
would also work. Though this solution is a bit workarounding because
MIPS_MAAR_ADDR wouldn't reflect the real mask of the ADDR field. Something
like the next macro would work better:

+#define MIPS_MAAR_ADDR \
+({ \
+ u64 __mask; \
+ \
+ if (cpu_has_lpa && read_c0_pagegrain() & PG_ELPA) { \
+ __mask = GENMASK_ULL(55, 12); \
+ else \
+ __mask = GENMASK_ULL(31, 12); \
+ \
+ __mask; \
+})

What do you think? What is better: the macro above or setting
GENMASK_ULL(55, 12)?

BTW I've just figured out, that since XPA is currently only supported by
kernels with CPU_MIPS32x config enabled, then only MIPS32 may have extended
physical addressing of 2^60 bytes if CONFIG_XPA is enabled. Generic MIPS64
doesn't support the extended phys addressing so only 2^36 bytes are available
on such platforms. (Loongson64 doesn't count, the platform code sets the
PG_ELPA bit manually in kernel-entry-init.h)

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-07 21:22:02

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 07/20] mips: Add MIPS Warrior P5600 support

On Thu, May 07, 2020 at 01:17:35PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:25PM +0300, [email protected] wrote:
> >
> > +config CPU_P5600
> > + bool "MIPS Warrior P5600"
> > + depends on SYS_HAS_CPU_P5600
> > + select CPU_HAS_PREFETCH
> > + select CPU_SUPPORTS_32BIT_KERNEL
> > + select CPU_SUPPORTS_HIGHMEM
> > + select CPU_SUPPORTS_MSA
> > + select CPU_SUPPORTS_UNCACHED_ACCELERATED
> > + select CPU_SUPPORTS_CPUFREQ
> > + select CPU_MIPSR2_IRQ_VI
> > + select CPU_MIPSR2_IRQ_EI
> > + select HAVE_KVM
> > + select MIPS_O32_FP64_SUPPORT
> > + help
> > + Choose this option to build a kernel for MIPS Warrior P5600 CPU.
> > + It's based on MIPS32r5 ISA with XPA, EVA, dual/quad issue exec pipes,
> > + MMU with two-levels TLB, UCA, MSA, MDU core level features and system
> > + level features like up to six P5600 calculation cores, CM2 with L2
> > + cache, IOCU/IOMMU (though might be unused depending on the system-
> > + specific IP core configuration), GIC, CPC, virtualisation module,
> > + eJTAG and PDtrace.
> > +
> > config CPU_R3000
> > bool "R3000"
> > depends on SYS_HAS_CPU_R3000
> > @@ -1841,7 +1863,8 @@ endchoice
> > config CPU_MIPS32_3_5_FEATURES
> > bool "MIPS32 Release 3.5 Features"
> > depends on SYS_HAS_CPU_MIPS32_R3_5
> > - depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6
> > + depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_MIPS32_R6 || \
> > + CPU_P5600
> > help
> > Choose this option to build a kernel for release 2 or later of the
> > MIPS32 architecture including features from the 3.5 release such as
> > @@ -1861,7 +1884,7 @@ config CPU_MIPS32_3_5_EVA
> > config CPU_MIPS32_R5_FEATURES
> > bool "MIPS32 Release 5 Features"
> > depends on SYS_HAS_CPU_MIPS32_R5
> > - depends on CPU_MIPS32_R2 || CPU_MIPS32_R5
> > + depends on CPU_MIPS32_R2 || CPU_MIPS32_R5 || CPU_P5600
> > help
> > Choose this option to build a kernel for release 2 or later of the
> > MIPS32 architecture including features from release 5 such as
> > @@ -2016,6 +2039,10 @@ config SYS_HAS_CPU_MIPS64_R6
> > bool
> > select ARCH_HAS_SYNC_DMA_FOR_CPU if DMA_NONCOHERENT
> >
> > +config SYS_HAS_CPU_P5600
> > + bool
> > + select ARCH_HAS_SYNC_DMA_FOR_CPU if DMA_NONCOHERENT
> > +
>
> P5600 is CPU_MIPS_R5 so can't you select it here and drop all the || CPU_5600
> above/below ?

Alas, We can't do this so easy. CONFIG_CPU_MIPS32_{R2,R5,R6} and any other
CONFIG_CPU_* configs is something that kernel config-file is supposed to select.
Their availability is enabled by the CONFIG_SYS_HAS_CPU_* configs. CONFIG_CPU_*
is supposed to activate CPU-specific features and there is only one
CONFIG_CPU_x can be enabled at a time seeing it's a part of the "CPU type"
choice kconfig menu. In addition the CPU config also tunes a compiler to activate
the arch-specific ISA and optimizations in the arch/mips/Makefile by setting
-march=cpu-name (where cpu-name can be p5600, mips32r5, etc).

Yes, P5600 is based on the MIPS32r5, but it also has got some specific features
(see config CPU_P5600 and config MIPS32_R5), which makes it to be different from
the ancestor. So In addition to the difficulties described above IMHO converting
CPU_P5600 to a set of features activated on top of the CPU_MIPS32_R5 config
would contradict the design of the CPU-support configs implemented in the MIPS
arch subsystem.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-07 21:34:15

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 04/20] mips: cm: Fix an invalid error code of INTVN_*_ERR

On Thu, May 07, 2020 at 01:10:07PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:22PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > Commit 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache
> > errors") adds cm2_causes[] array with map of error type ID and
> > pointers to the short description string. There is a mistake in
> > the table, since according to MIPS32 manual CM2_ERROR_TYPE = {17,18}
> > correspond to INTVN_WR_ERR and INTVN_RD_ERR, while the table
> > claims they have {0x17,0x18} codes. This is obviously hex-dec
> > copy-paste bug. Moreover codes {0x18 - 0x1a} indicate L2 ECC errors.
> >
> > Fixes: 3885c2b463f6 ("MIPS: CM: Add support for reporting CM cache errors")
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > arch/mips/kernel/mips-cm.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> applied to mips-next.

Great! Thanks.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-07 21:42:08

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 05/20] mips: cm: Add L2 ECC/parity errors reporting

On Thu, May 07, 2020 at 01:17:53PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:23PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > According to the MIPS32 InterAptiv software manual error codes 24 - 26
> > of CM2 indicate L2 ECC/parity error with switching to a corresponding
> > errors info fields. This patch provides these errors parsing code,
> > which handles the read/write uncorrectable and correctable ECC/parity
> > errors, and prints instruction causing the fault, RAM array type, cache
> > way/dword and syndrome associated with the faulty data.
> >
> > Co-developed-by: Alexey Malahov <[email protected]>
> > Signed-off-by: Alexey Malahov <[email protected]>
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > arch/mips/kernel/mips-cm.c | 62 ++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 60 insertions(+), 2 deletions(-)
>
> applied to mips-next.

Great! Thanks.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-08 09:35:01

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 11/20] mips: MAAR: Use more precise address mask

On Thu, May 07, 2020 at 10:13:37PM +0300, Serge Semin wrote:
> On Thu, May 07, 2020 at 01:09:51PM +0200, Thomas Bogendoerfer wrote:
> > On Wed, May 06, 2020 at 08:42:29PM +0300, [email protected] wrote:
> > > From: Serge Semin <[email protected]>
> > >
> > > Indeed according to the P5600/P6000 manual the MAAR pair register
> > > address field either takes [12:31] bits for 32-bits non-XPA systems
> > > and [12:35] otherwise. In any case the current address mask is just
> > > wrong for 64-bit and 32-bits XPA chips. So lets extend it to 39-bits
> > > value. This shall cover the 64-bits architecture and systems with XPA
> > > enabled, and won't cause any problem for non-XPA 32-bit systems, since
> > > the value will be just truncated when written to the 32-bits register.
> >
> > according to MIPS32 Priveleged Resoure Architecture Rev. 6.02
> > ADDR spans from bit 12 to bit 55. So your patch fits only for P5600.
>
> > Does the wider mask cause any problems ?
>
> No, it won't. Bits written to the [40:62] range will be just ignored,
> while reading from there should return zeros. Setting GENMASK_ULL(55, 12)
> would also work. Though this solution is a bit workarounding because
> MIPS_MAAR_ADDR wouldn't reflect the real mask of the ADDR field. Something
> like the next macro would work better:
>
> +#define MIPS_MAAR_ADDR \
> +({ \
> + u64 __mask; \
> + \
> + if (cpu_has_lpa && read_c0_pagegrain() & PG_ELPA) { \
> + __mask = GENMASK_ULL(55, 12); \
> + else \
> + __mask = GENMASK_ULL(31, 12); \
> + \
> + __mask; \
> +})

that looks horrible.

> What do you think? What is better: the macro above or setting
> GENMASK_ULL(55, 12)?

just that one ;-)

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-08 09:35:05

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 07/20] mips: Add MIPS Warrior P5600 support

On Fri, May 08, 2020 at 12:19:23AM +0300, Serge Semin wrote:
> On Thu, May 07, 2020 at 01:17:35PM +0200, Thomas Bogendoerfer wrote:
> > P5600 is CPU_MIPS_R5 so can't you select it here and drop all the || CPU_5600
> > above/below ?
>
> Alas, We can't do this so easy. CONFIG_CPU_MIPS32_{R2,R5,R6} and any other
> CONFIG_CPU_* configs is something that kernel config-file is supposed to select.
> Their availability is enabled by the CONFIG_SYS_HAS_CPU_* configs. CONFIG_CPU_*
> is supposed to activate CPU-specific features and there is only one
> CONFIG_CPU_x can be enabled at a time seeing it's a part of the "CPU type"
> choice kconfig menu. In addition the CPU config also tunes a compiler to activate
> the arch-specific ISA and optimizations in the arch/mips/Makefile by setting
> -march=cpu-name (where cpu-name can be p5600, mips32r5, etc).
>
> Yes, P5600 is based on the MIPS32r5, but it also has got some specific features
> (see config CPU_P5600 and config MIPS32_R5), which makes it to be different from
> the ancestor. So In addition to the difficulties described above IMHO converting
> CPU_P5600 to a set of features activated on top of the CPU_MIPS32_R5 config
> would contradict the design of the CPU-support configs implemented in the MIPS
> arch subsystem.

maybe I wasn't clear enough, my suggestion is

use

config CPU_P5600
bool "MIPS Warrior P5600"
depends on SYS_HAS_CPU_P5600
select CPU_MIPS32_R5
select CPU_SUPPORTS_UNCACHED_ACCELERATED
select CPU_SUPPORTS_CPUFREQ
select CPU_MIPSR2_IRQ_VI
select CPU_MIPSR2_IRQ_EI

That way you don't need to any "|| CPU_P5600" where CPU_MIPS32_R5 is
already there. Or are there cases, where this would be wrong ?

Thomas.


--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-08 12:20:33

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v2 17/20] mips: Add udelay lpj numbers adjustment

On Wed, 6 May 2020 20:42:35 +0300
<[email protected]> wrote:

> From: Serge Semin <[email protected]>
>
> Loops-per-jiffies is a special number which represents a number of
> noop-loop cycles per CPU-scheduler quantum - jiffies. As you
> understand aside from CPU-specific implementation it depends on
> the CPU frequency. So when a platform has the CPU frequency fixed,
> we have no problem and the current udelay interface will work
> just fine. But as soon as CPU-freq driver is enabled and the cores
> frequency changes, we'll end up with distorted udelay's. In order
> to fix this we have to accordinly adjust the per-CPU udelay_val
> (the same as the global loops_per_jiffy) number. This can be done
> in the CPU-freq transition event handler. We subscribe to that event
> in the MIPS arch time-inititalization method.
>
> Co-developed-by: Alexey Malahov <[email protected]>
> Signed-off-by: Alexey Malahov <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]

Reviewed-by: Jiaxun Yang <[email protected]>

That have been absent in MIPS kernel so long!

Thanks.
> ---
[...]
---
Jiaxun Yang

2020-05-08 12:32:41

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 07/20] mips: Add MIPS Warrior P5600 support

On Fri, May 08, 2020 at 11:32:59AM +0200, Thomas Bogendoerfer wrote:
> On Fri, May 08, 2020 at 12:19:23AM +0300, Serge Semin wrote:
> > On Thu, May 07, 2020 at 01:17:35PM +0200, Thomas Bogendoerfer wrote:
> > > P5600 is CPU_MIPS_R5 so can't you select it here and drop all the || CPU_5600
> > > above/below ?
> >
> > Alas, We can't do this so easy. CONFIG_CPU_MIPS32_{R2,R5,R6} and any other
> > CONFIG_CPU_* configs is something that kernel config-file is supposed to select.
> > Their availability is enabled by the CONFIG_SYS_HAS_CPU_* configs. CONFIG_CPU_*
> > is supposed to activate CPU-specific features and there is only one
> > CONFIG_CPU_x can be enabled at a time seeing it's a part of the "CPU type"
> > choice kconfig menu. In addition the CPU config also tunes a compiler to activate
> > the arch-specific ISA and optimizations in the arch/mips/Makefile by setting
> > -march=cpu-name (where cpu-name can be p5600, mips32r5, etc).
> >
> > Yes, P5600 is based on the MIPS32r5, but it also has got some specific features
> > (see config CPU_P5600 and config MIPS32_R5), which makes it to be different from
> > the ancestor. So In addition to the difficulties described above IMHO converting
> > CPU_P5600 to a set of features activated on top of the CPU_MIPS32_R5 config
> > would contradict the design of the CPU-support configs implemented in the MIPS
> > arch subsystem.
>
> maybe I wasn't clear enough, my suggestion is
>
> use
>
> config CPU_P5600
> bool "MIPS Warrior P5600"
> depends on SYS_HAS_CPU_P5600
> select CPU_MIPS32_R5
> select CPU_SUPPORTS_UNCACHED_ACCELERATED
> select CPU_SUPPORTS_CPUFREQ
> select CPU_MIPSR2_IRQ_VI
> select CPU_MIPSR2_IRQ_EI
>
> That way you don't need to any "|| CPU_P5600" where CPU_MIPS32_R5 is
> already there. Or are there cases, where this would be wrong ?

nevermind, this would also need a select SYS_HAS_CPU_MIPS32_R5, which
isn't wanted here. So patch is fine as is.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-08 13:34:46

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 06/20] mips: Add MIPS32 Release 5 support

On Wed, May 06, 2020 at 08:42:24PM +0300, [email protected] wrote:
> We also could add CPU_MIPS64_R5 config support here, but I don't think
> it's necessary at the moment seeing there is no any real chip ever
> produced with that arch. Right?

how much is missing ? Looks like not too much, so it might be worth
to add it at least for symmetry to the other ISAs...

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-08 13:36:05

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 08/20] mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs

On Wed, May 06, 2020 at 08:42:26PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> index e2f31bd6363b..7e22b9c1e279 100644
> --- a/arch/mips/include/asm/cpu-features.h
> +++ b/arch/mips/include/asm/cpu-features.h
> @@ -64,6 +64,8 @@
> ((MIPS_ISA_REV >= (ge)) && (MIPS_ISA_REV < (lt)))
> #define __isa_range_or_flag(ge, lt, flag) \
> (__isa_range(ge, lt) || ((MIPS_ISA_REV < (lt)) && __isa(flag)))
> +#define __isa_range_and_flag(ge, lt, flag) \
> + (__isa_range(ge, lt) && __isa(flag))
>
> /*
> * SMP assumption: Options of CPU 0 are a superset of all processors.
> @@ -291,10 +293,10 @@
> # define cpu_has_mips32r6 __isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
> #endif
> #ifndef cpu_has_mips64r1
> -# define cpu_has_mips64r1 __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1)
> +# define cpu_has_mips64r1 __isa_range_and_flag(1, 6, MIPS_CPU_ISA_M64R1)

that's not the correct fix. You want to check for cpu_has_64bits here.
Something like

# define cpu_has_mips64r1 (cpu_has_64bits && __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1))

should do the trick.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-08 15:46:33

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 19/20] mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU

On Wed, May 06, 2020 at 08:42:37PM +0300, [email protected] wrote:
> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> index 17a9cbb8b3df..f5b72fb7d5ee 100644
> --- a/arch/mips/kernel/cevt-r4k.c
> +++ b/arch/mips/kernel/cevt-r4k.c
> @@ -8,6 +8,7 @@
> */
> #include <linux/clockchips.h>
> #include <linux/interrupt.h>
> +#include <linux/cpufreq.h>
> #include <linux/percpu.h>
> #include <linux/smp.h>
> #include <linux/irq.h>
> @@ -250,6 +251,49 @@ unsigned int __weak get_c0_compare_int(void)
> return MIPS_CPU_IRQ_BASE + cp0_compare_irq;
> }
>
> +#ifdef CONFIG_CPU_FREQ
> +
> +static unsigned long mips_ref_freq;
> +
> +static int cpufreq_callback(struct notifier_block *nb,
> + unsigned long val, void *data)

please prefix function names with r4k_ to make them different from
the other ones you implemented in kernel/time.c. I know they are
static, but keeping different names makes looking at crashes easier.

> + struct cpufreq_freqs *freq = data;
> + struct clock_event_device *cd;
> + unsigned long rate;
> + int cpu;
> +
> + if (!mips_ref_freq)
> + mips_ref_freq = freq->old;

isn't this the same as mips_hpt_frequency ?

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-08 15:46:33

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 06, 2020 at 08:42:36PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> Commit 07d69579e7fe ("MIPS: Don't register r4k sched clock when
> CPUFREQ enabled") disabled the r4k-clock usage for scheduler ticks
> counting due to the scheduler being non-tolerant for unstable
> clocks sources. For the same reason the clock should be used
> in the system clocksource framework only as a last resort if CPU
> frequency may change.
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/mips/kernel/csrc-r4k.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
> index 437dda64fd7a..d81fb374f477 100644
> --- a/arch/mips/kernel/csrc-r4k.c
> +++ b/arch/mips/kernel/csrc-r4k.c
> @@ -71,7 +71,11 @@ int __init init_r4k_clocksource(void)
> return -ENXIO;
>
> /* Calculate a somewhat reasonable rating value */
> +#ifndef CONFIG_CPU_FREQ
> clocksource_mips.rating = 200 + mips_hpt_frequency / 10000000;
> +#else
> + clocksource_mips.rating = 99;
> +#endif

I dislike this patch. Assuming you have an other clocksource, why not
simply disable csrc-r4k, if CPU_FREQ is enabled ?

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-10 22:12:54

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 06/20] mips: Add MIPS32 Release 5 support

On Fri, May 08, 2020 at 03:30:40PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:24PM +0300, [email protected] wrote:
> > We also could add CPU_MIPS64_R5 config support here, but I don't think
> > it's necessary at the moment seeing there is no any real chip ever
> > produced with that arch. Right?
>
> how much is missing ? Looks like not too much, so it might be worth
> to add it at least for symmetry to the other ISAs...

Yeah, just a few more alteration to add together with new CPU_MIPS64_R5 config.
I'll do this in v3.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-10 22:12:54

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 07/20] mips: Add MIPS Warrior P5600 support

On Fri, May 08, 2020 at 02:21:37PM +0200, Thomas Bogendoerfer wrote:
> On Fri, May 08, 2020 at 11:32:59AM +0200, Thomas Bogendoerfer wrote:
> > On Fri, May 08, 2020 at 12:19:23AM +0300, Serge Semin wrote:
> > > On Thu, May 07, 2020 at 01:17:35PM +0200, Thomas Bogendoerfer wrote:
> > > > P5600 is CPU_MIPS_R5 so can't you select it here and drop all the || CPU_5600
> > > > above/below ?
> > >
> > > Alas, We can't do this so easy. CONFIG_CPU_MIPS32_{R2,R5,R6} and any other
> > > CONFIG_CPU_* configs is something that kernel config-file is supposed to select.
> > > Their availability is enabled by the CONFIG_SYS_HAS_CPU_* configs. CONFIG_CPU_*
> > > is supposed to activate CPU-specific features and there is only one
> > > CONFIG_CPU_x can be enabled at a time seeing it's a part of the "CPU type"
> > > choice kconfig menu. In addition the CPU config also tunes a compiler to activate
> > > the arch-specific ISA and optimizations in the arch/mips/Makefile by setting
> > > -march=cpu-name (where cpu-name can be p5600, mips32r5, etc).
> > >
> > > Yes, P5600 is based on the MIPS32r5, but it also has got some specific features
> > > (see config CPU_P5600 and config MIPS32_R5), which makes it to be different from
> > > the ancestor. So In addition to the difficulties described above IMHO converting
> > > CPU_P5600 to a set of features activated on top of the CPU_MIPS32_R5 config
> > > would contradict the design of the CPU-support configs implemented in the MIPS
> > > arch subsystem.
> >
> > maybe I wasn't clear enough, my suggestion is
> >
> > use
> >
> > config CPU_P5600
> > bool "MIPS Warrior P5600"
> > depends on SYS_HAS_CPU_P5600
> > select CPU_MIPS32_R5
> > select CPU_SUPPORTS_UNCACHED_ACCELERATED
> > select CPU_SUPPORTS_CPUFREQ
> > select CPU_MIPSR2_IRQ_VI
> > select CPU_MIPSR2_IRQ_EI
> >
> > That way you don't need to any "|| CPU_P5600" where CPU_MIPS32_R5 is
> > already there. Or are there cases, where this would be wrong ?
>
> nevermind, this would also need a select SYS_HAS_CPU_MIPS32_R5, which
> isn't wanted here. So patch is fine as is.

Ok. Thanks.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-10 22:15:36

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 11/20] mips: MAAR: Use more precise address mask

On Fri, May 08, 2020 at 11:22:36AM +0200, Thomas Bogendoerfer wrote:
> On Thu, May 07, 2020 at 10:13:37PM +0300, Serge Semin wrote:
> > On Thu, May 07, 2020 at 01:09:51PM +0200, Thomas Bogendoerfer wrote:
> > > On Wed, May 06, 2020 at 08:42:29PM +0300, [email protected] wrote:
> > > > From: Serge Semin <[email protected]>
> > > >
> > > > Indeed according to the P5600/P6000 manual the MAAR pair register
> > > > address field either takes [12:31] bits for 32-bits non-XPA systems
> > > > and [12:35] otherwise. In any case the current address mask is just
> > > > wrong for 64-bit and 32-bits XPA chips. So lets extend it to 39-bits
> > > > value. This shall cover the 64-bits architecture and systems with XPA
> > > > enabled, and won't cause any problem for non-XPA 32-bit systems, since
> > > > the value will be just truncated when written to the 32-bits register.
> > >
> > > according to MIPS32 Priveleged Resoure Architecture Rev. 6.02
> > > ADDR spans from bit 12 to bit 55. So your patch fits only for P5600.
> >
> > > Does the wider mask cause any problems ?
> >
> > No, it won't. Bits written to the [40:62] range will be just ignored,
> > while reading from there should return zeros. Setting GENMASK_ULL(55, 12)
> > would also work. Though this solution is a bit workarounding because
> > MIPS_MAAR_ADDR wouldn't reflect the real mask of the ADDR field. Something
> > like the next macro would work better:
> >
> > +#define MIPS_MAAR_ADDR \
> > +({ \
> > + u64 __mask; \
> > + \
> > + if (cpu_has_lpa && read_c0_pagegrain() & PG_ELPA) { \
> > + __mask = GENMASK_ULL(55, 12); \
> > + else \
> > + __mask = GENMASK_ULL(31, 12); \
> > + \
> > + __mask; \
> > +})
>
> that looks horrible.
>
> > What do you think? What is better: the macro above or setting
> > GENMASK_ULL(55, 12)?
>
> just that one ;-)

Agreed. I'll fix it in v3.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-11 00:05:02

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 08/20] mips: Fix cpu_has_mips64r1/2 activation for MIPS32 CPUs

On Fri, May 08, 2020 at 03:28:09PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:26PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> > diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> > index e2f31bd6363b..7e22b9c1e279 100644
> > --- a/arch/mips/include/asm/cpu-features.h
> > +++ b/arch/mips/include/asm/cpu-features.h
> > @@ -64,6 +64,8 @@
> > ((MIPS_ISA_REV >= (ge)) && (MIPS_ISA_REV < (lt)))
> > #define __isa_range_or_flag(ge, lt, flag) \
> > (__isa_range(ge, lt) || ((MIPS_ISA_REV < (lt)) && __isa(flag)))
> > +#define __isa_range_and_flag(ge, lt, flag) \
> > + (__isa_range(ge, lt) && __isa(flag))
> >
> > /*
> > * SMP assumption: Options of CPU 0 are a superset of all processors.
> > @@ -291,10 +293,10 @@
> > # define cpu_has_mips32r6 __isa_ge_or_flag(6, MIPS_CPU_ISA_M32R6)
> > #endif
> > #ifndef cpu_has_mips64r1
> > -# define cpu_has_mips64r1 __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1)
> > +# define cpu_has_mips64r1 __isa_range_and_flag(1, 6, MIPS_CPU_ISA_M64R1)
>
> that's not the correct fix. You want to check for cpu_has_64bits here.
> Something like
>
> # define cpu_has_mips64r1 (cpu_has_64bits && __isa_range_or_flag(1, 6, MIPS_CPU_ISA_M64R1))
>
> should do the trick.

Good point. Thanks. I'll fix it in v3.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-11 00:38:39

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 19/20] mips: cevt-r4k: Update the r4k-clockevent frequency in sync with CPU

On Fri, May 08, 2020 at 05:40:46PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:37PM +0300, [email protected] wrote:
> > diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> > index 17a9cbb8b3df..f5b72fb7d5ee 100644
> > --- a/arch/mips/kernel/cevt-r4k.c
> > +++ b/arch/mips/kernel/cevt-r4k.c
> > @@ -8,6 +8,7 @@
> > */
> > #include <linux/clockchips.h>
> > #include <linux/interrupt.h>
> > +#include <linux/cpufreq.h>
> > #include <linux/percpu.h>
> > #include <linux/smp.h>
> > #include <linux/irq.h>
> > @@ -250,6 +251,49 @@ unsigned int __weak get_c0_compare_int(void)
> > return MIPS_CPU_IRQ_BASE + cp0_compare_irq;
> > }
> >
> > +#ifdef CONFIG_CPU_FREQ
> > +
> > +static unsigned long mips_ref_freq;
> > +
> > +static int cpufreq_callback(struct notifier_block *nb,
> > + unsigned long val, void *data)
>
> please prefix function names with r4k_ to make them different from
> the other ones you implemented in kernel/time.c. I know they are
> static, but keeping different names makes looking at crashes easier.

Agreed. I'll fix it in v3.

>
> > + struct cpufreq_freqs *freq = data;
> > + struct clock_event_device *cd;
> > + unsigned long rate;
> > + int cpu;
> > +
> > + if (!mips_ref_freq)
> > + mips_ref_freq = freq->old;
>
> isn't this the same as mips_hpt_frequency ?

No. Here I save the initial CPU frequency so use one then to scale the
mips_hpt_frequency value in accordance with the CPU clock rate change. Yes,
mips_hpt_frequency value may initially match the CPU frequency on some platforms
but normally the r4k timer is clocked with half of it while some systems may have
a complicated algorithm of the timer ref clock rate calculation.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-11 13:33:23

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Fri, May 08, 2020 at 05:41:50PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:36PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > Commit 07d69579e7fe ("MIPS: Don't register r4k sched clock when
> > CPUFREQ enabled") disabled the r4k-clock usage for scheduler ticks
> > counting due to the scheduler being non-tolerant for unstable
> > clocks sources. For the same reason the clock should be used
> > in the system clocksource framework only as a last resort if CPU
> > frequency may change.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > arch/mips/kernel/csrc-r4k.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
> > index 437dda64fd7a..d81fb374f477 100644
> > --- a/arch/mips/kernel/csrc-r4k.c
> > +++ b/arch/mips/kernel/csrc-r4k.c
> > @@ -71,7 +71,11 @@ int __init init_r4k_clocksource(void)
> > return -ENXIO;
> >
> > /* Calculate a somewhat reasonable rating value */
> > +#ifndef CONFIG_CPU_FREQ
> > clocksource_mips.rating = 200 + mips_hpt_frequency / 10000000;
> > +#else
> > + clocksource_mips.rating = 99;
> > +#endif
>
> I dislike this patch. Assuming you have an other clocksource, why not
> simply disable csrc-r4k, if CPU_FREQ is enabled ?

Me neither and the best way would be to update the clocksource frequency
dynamically the same way it's done for cevt-r4k and MIPS GIC timers. Alas the
clocksource doesn't support it. Due to this together with CPU-freq facility
enabled we have to use a very slow DW APB Timer instead of the fast embedded
into the CPU core r4k and MIPS GIC timers. Just note the difference: it takes
220 ns to read the counter from DW APB Timer in comparison to a few nanoseconds
reading from MIPS GIC and R4K. So IMO disabling the timer as you suggest isn't
the best option. By making the CPUFREQ and CSRC_R4K mutual exclusive we'd
assume a use-case that the system will always use the CPU-freq facility changing
the CPU reference frequency. This is obviously not true. Noone prevents the
system administrator to leave the default setting of the CPU-freq with fixed
frequency and select a faster, more accurate timer like in our case.

My idea was not to try to predict how the system would be used, but to let the
system administration to choose which timer is applicable in particular usecase
enabling a safest one by default. So if CPUFREQ is available, then we fallback
to the external timer as safest one. If the system user wouldn't need to have
the CPUFREQ facility utilized, then the system administrator would want to
leave the default CPU-freq governor with pre-defined CPU frequency and
select either R4K (MIPS) or MIPS GIC timer just by writing its name into
/sys/bus/clocksource/devices/clocksource0/current_clocksource .

I should note, that currently CPU_FREQ won't be available if there is no
MIPS_EXTERNAL_TIMER available for the platform. It's prohibited by means of the
conditional kbuild config inclusion declared in the arch/mips/Kconfig:
+ if CPU_SUPPORTS_CPUFREQ && MIPS_EXTERNAL_TIMER
+ source "drivers/cpufreq/Kconfig"
+ endif
So if there is no external timer working independently from the CPU core clock
source, the CPUFREQ won't be available to select for the kernel. Though currently
this limitation is supposed to be applicable for the R4K/MIPS GIC clocksource
timers only since clockevents must work fine in unstable reference clock conditions.

So what can we do to improve the patch? First one is a solution I suggested in
this patch but it could be a bit altered by using IS_ENABLED() macro to:
+ clocksource_mips.rating = !IS_ENABLED(CONFIG_CPU_FREQ) ?
+ 200 + mips_hpt_frequency / 10000000 : 99;

Another idea I discovered when have been searching through the x86 arch code.
x86's got the same problem with TSC timer, but it doesn't disable it if
CPU-frequency is switched on. Instead it just marks it as unstable by calling
the clocksource_mark_unstable() method if CPU frequency changes. I suggest to
implement the same approach in our case of MIPS GIC (another patchset
I've sent, see "clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC
support" in your email client) and R4K timers. We'll subscribe to the CPU
frequency change and if it changes we'll call clocksource_mark_unstable() with
MIPS GIC and R4K clocksource handlers passed. This shall reduce their rating and
cause selecting a clocksource with better one. BTW I suppose it won't be
necessary to initially lower the rating of the MIPS GIC and R4K clocksource
timers if this is implemented.

So, what do you think?

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-14 15:14:06

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 02/20] dt-bindings: bus: Add MIPS CDMM controller

On Wed, 6 May 2020 20:42:20 +0300, wrote:
> From: Serge Semin <[email protected]>
>
> It's a Common Device Memory Map controller embedded into the MIPS IP
> cores, which dts node is supposed to have compatible and reg properties.
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---
>
> Changelog v2:
> - Lowercase the example hex'es.
> ---
> .../bindings/bus/mti,mips-cdmm.yaml | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml
>

Reviewed-by: Rob Herring <[email protected]>

2020-05-14 18:08:13

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 02/20] dt-bindings: bus: Add MIPS CDMM controller

On Thu, May 14, 2020 at 10:09:43AM -0500, Rob Herring wrote:
> On Wed, 6 May 2020 20:42:20 +0300, wrote:
> > From: Serge Semin <[email protected]>
> >
> > It's a Common Device Memory Map controller embedded into the MIPS IP
> > cores, which dts node is supposed to have compatible and reg properties.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > ---
> >
> > Changelog v2:
> > - Lowercase the example hex'es.
> > ---
> > .../bindings/bus/mti,mips-cdmm.yaml | 35 +++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/bus/mti,mips-cdmm.yaml
> >
>
> Reviewed-by: Rob Herring <[email protected]>

Great! Thanks.

-Sergey

2020-05-14 18:15:18

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

Rob,

Could you also take a look at this patch? There are several patchsets I've sent
which depend on the vendor-prefix it defines. So when you get to check those
patchsets DT files, the dt_binding_check will fail without it. Is it possible
somehow to pick this patch up from here and apply it before checking those
Baikal-T1-specific binding files?

-Sergey

On Wed, May 06, 2020 at 08:42:21PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> Add "BAIKAL ELECTRONICS, JSC" to the list of devicetree vendor prefixes
> as "baikal".
>
> Website: http://www.baikalelectronics.com
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---
>
> Changelog v2:
> - Fix author and SoB emails mismatch.
> - Add 'baikal' vendor prefix instead of ambiguous 'be'.
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index d3891386d671..674c0d07c0ad 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -139,6 +139,8 @@ patternProperties:
> description: Azoteq (Pty) Ltd
> "^azw,.*":
> description: Shenzhen AZW Technology Co., Ltd.
> + "^baikal,.*":
> + description: BAIKAL ELECTRONICS, JSC
> "^bananapi,.*":
> description: BIPAI KEJI LIMITED
> "^beacon,.*":
> --
> 2.25.1
>

2020-05-14 18:34:46

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] dt-bindings: Add vendor prefix for Baikal Electronics, JSC

On Wed, 6 May 2020 20:42:21 +0300, wrote:
> From: Serge Semin <[email protected]>
>
> Add "BAIKAL ELECTRONICS, JSC" to the list of devicetree vendor prefixes
> as "baikal".
>
> Website: http://www.baikalelectronics.com
>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> ---
>
> Changelog v2:
> - Fix author and SoB emails mismatch.
> - Add 'baikal' vendor prefix instead of ambiguous 'be'.
> ---
> Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>

Applied, thanks!

2020-05-15 07:50:40

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

Thomas,
Could you take a look at my comment below so I could proceed with the
patchset v3 development?

-Sergey

On Mon, May 11, 2020 at 04:31:21PM +0300, Serge Semin wrote:
> On Fri, May 08, 2020 at 05:41:50PM +0200, Thomas Bogendoerfer wrote:
> > On Wed, May 06, 2020 at 08:42:36PM +0300, [email protected] wrote:
> > > From: Serge Semin <[email protected]>
> > >
> > > Commit 07d69579e7fe ("MIPS: Don't register r4k sched clock when
> > > CPUFREQ enabled") disabled the r4k-clock usage for scheduler ticks
> > > counting due to the scheduler being non-tolerant for unstable
> > > clocks sources. For the same reason the clock should be used
> > > in the system clocksource framework only as a last resort if CPU
> > > frequency may change.
> > >
> > > Signed-off-by: Serge Semin <[email protected]>
> > > Cc: Alexey Malahov <[email protected]>
> > > Cc: Thomas Bogendoerfer <[email protected]>
> > > Cc: Paul Burton <[email protected]>
> > > Cc: Ralf Baechle <[email protected]>
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: Arnd Bergmann <[email protected]>
> > > Cc: Rob Herring <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > ---
> > > arch/mips/kernel/csrc-r4k.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
> > > index 437dda64fd7a..d81fb374f477 100644
> > > --- a/arch/mips/kernel/csrc-r4k.c
> > > +++ b/arch/mips/kernel/csrc-r4k.c
> > > @@ -71,7 +71,11 @@ int __init init_r4k_clocksource(void)
> > > return -ENXIO;
> > >
> > > /* Calculate a somewhat reasonable rating value */
> > > +#ifndef CONFIG_CPU_FREQ
> > > clocksource_mips.rating = 200 + mips_hpt_frequency / 10000000;
> > > +#else
> > > + clocksource_mips.rating = 99;
> > > +#endif
> >
> > I dislike this patch. Assuming you have an other clocksource, why not
> > simply disable csrc-r4k, if CPU_FREQ is enabled ?
>
> Me neither and the best way would be to update the clocksource frequency
> dynamically the same way it's done for cevt-r4k and MIPS GIC timers. Alas the
> clocksource doesn't support it. Due to this together with CPU-freq facility
> enabled we have to use a very slow DW APB Timer instead of the fast embedded
> into the CPU core r4k and MIPS GIC timers. Just note the difference: it takes
> 220 ns to read the counter from DW APB Timer in comparison to a few nanoseconds
> reading from MIPS GIC and R4K. So IMO disabling the timer as you suggest isn't
> the best option. By making the CPUFREQ and CSRC_R4K mutual exclusive we'd
> assume a use-case that the system will always use the CPU-freq facility changing
> the CPU reference frequency. This is obviously not true. Noone prevents the
> system administrator to leave the default setting of the CPU-freq with fixed
> frequency and select a faster, more accurate timer like in our case.
>
> My idea was not to try to predict how the system would be used, but to let the
> system administration to choose which timer is applicable in particular usecase
> enabling a safest one by default. So if CPUFREQ is available, then we fallback
> to the external timer as safest one. If the system user wouldn't need to have
> the CPUFREQ facility utilized, then the system administrator would want to
> leave the default CPU-freq governor with pre-defined CPU frequency and
> select either R4K (MIPS) or MIPS GIC timer just by writing its name into
> /sys/bus/clocksource/devices/clocksource0/current_clocksource .
>
> I should note, that currently CPU_FREQ won't be available if there is no
> MIPS_EXTERNAL_TIMER available for the platform. It's prohibited by means of the
> conditional kbuild config inclusion declared in the arch/mips/Kconfig:
> + if CPU_SUPPORTS_CPUFREQ && MIPS_EXTERNAL_TIMER
> + source "drivers/cpufreq/Kconfig"
> + endif
> So if there is no external timer working independently from the CPU core clock
> source, the CPUFREQ won't be available to select for the kernel. Though currently
> this limitation is supposed to be applicable for the R4K/MIPS GIC clocksource
> timers only since clockevents must work fine in unstable reference clock conditions.
>
> So what can we do to improve the patch? First one is a solution I suggested in
> this patch but it could be a bit altered by using IS_ENABLED() macro to:
> + clocksource_mips.rating = !IS_ENABLED(CONFIG_CPU_FREQ) ?
> + 200 + mips_hpt_frequency / 10000000 : 99;
>
> Another idea I discovered when have been searching through the x86 arch code.
> x86's got the same problem with TSC timer, but it doesn't disable it if
> CPU-frequency is switched on. Instead it just marks it as unstable by calling
> the clocksource_mark_unstable() method if CPU frequency changes. I suggest to
> implement the same approach in our case of MIPS GIC (another patchset
> I've sent, see "clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC
> support" in your email client) and R4K timers. We'll subscribe to the CPU
> frequency change and if it changes we'll call clocksource_mark_unstable() with
> MIPS GIC and R4K clocksource handlers passed. This shall reduce their rating and
> cause selecting a clocksource with better one. BTW I suppose it won't be
> necessary to initially lower the rating of the MIPS GIC and R4K clocksource
> timers if this is implemented.
>
> So, what do you think?
>
> -Sergey
>
> >
> > Thomas.
> >
> > --
> > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> > good idea. [ RFC1925, 2.3 ]

2020-05-15 16:01:03

by Wysocki, Rafael J

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On 5/6/2020 7:42 PM, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> Recent commit e61a41256edf ("cpufreq: dev_pm_qos_update_request() can
> return 1 on success") fixed a problem when active policies traverse
> was falsely stopped due to invalidly treating the non-zero return value
> from freq_qos_update_request() method as an error. Yes, that function
> can return positive values if the requested update actually took place.
> The current problem is that the returned value is then passed to the
> return cell of the cpufreq_boost_set_sw() (set_boost callback) method.
> This value is then also analyzed for being non-zero, which is also
> treated as having an error. As a result during the boost activation
> we'll get an error returned while having the QOS frequency update
> successfully performed. Fix this by returning a negative value from the
> cpufreq_boost_set_sw() if actual error was encountered and zero
> otherwise treating any positive values as the successful operations
> completion.
>
> Fixes: 18c49926c4bf ("cpufreq: Add QoS requests for userspace constraints")
> Signed-off-by: Serge Semin <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>
> Cc: Alexey Malahov <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/cpufreq/cpufreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 045f9fe157ce..5870cdca88cf 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state)
> break;
> }
>
> - return ret;
> + return ret < 0 ? ret : 0;
> }
>
> int cpufreq_boost_trigger_state(int state)

IMO it is better to update the caller of this function to handle the
positive value possibly returned by it correctly.

Thanks!


2020-05-15 21:15:48

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> Thomas,
> Could you take a look at my comment below so I could proceed with the
> patchset v3 development?

I can't help, but using r4k clocksource with changing frequency is
probaly only usefull as a random generator. So IMHO the only two
options are disabling it or implement what arch/x86/kernel/tsc.c does.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-16 11:59:19

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > Thomas,
> > Could you take a look at my comment below so I could proceed with the
> > patchset v3 development?
>
> I can't help, but using r4k clocksource with changing frequency is
> probaly only usefull as a random generator. So IMHO the only two
> options are disabling it or implement what arch/x86/kernel/tsc.c does.

Then it's settled. I'll resend the series with csrc-r4k updated to have the
tsc-like design implemented.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-16 12:55:50

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

Hello Rafael,

On Fri, May 15, 2020 at 05:58:47PM +0200, Rafael J. Wysocki wrote:
> On 5/6/2020 7:42 PM, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > Recent commit e61a41256edf ("cpufreq: dev_pm_qos_update_request() can
> > return 1 on success") fixed a problem when active policies traverse
> > was falsely stopped due to invalidly treating the non-zero return value
> > from freq_qos_update_request() method as an error. Yes, that function
> > can return positive values if the requested update actually took place.
> > The current problem is that the returned value is then passed to the
> > return cell of the cpufreq_boost_set_sw() (set_boost callback) method.
> > This value is then also analyzed for being non-zero, which is also
> > treated as having an error. As a result during the boost activation
> > we'll get an error returned while having the QOS frequency update
> > successfully performed. Fix this by returning a negative value from the
> > cpufreq_boost_set_sw() if actual error was encountered and zero
> > otherwise treating any positive values as the successful operations
> > completion.
> >
> > Fixes: 18c49926c4bf ("cpufreq: Add QoS requests for userspace constraints")
> > Signed-off-by: Serge Semin <[email protected]>
> > Acked-by: Viresh Kumar <[email protected]>
> > Cc: Alexey Malahov <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/cpufreq/cpufreq.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 045f9fe157ce..5870cdca88cf 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state)
> > break;
> > }
> > - return ret;
> > + return ret < 0 ? ret : 0;
> > }
> > int cpufreq_boost_trigger_state(int state)
>
> IMO it is better to update the caller of this function to handle the
> positive value possibly returned by it correctly.

Could you elaborate why? Viresh seems to be ok with this solution.

As I see it the caller doesn't expect the positive value returned by the
original freq_qos_update_request(). It just doesn't need to know whether the
effective policy has been updated or not, it only needs to make sure the
operations has been successful. Moreover the positive value is related only
to the !last! active policy, which doesn't give the caller a full picture
of the policy change anyway. So taking all of these into account I'd leave the
fix as is.

Regards,
-Sergey

>
> Thanks!
>
>

2020-05-18 07:43:34

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On 16-05-20, 15:52, Serge Semin wrote:
> On Fri, May 15, 2020 at 05:58:47PM +0200, Rafael J. Wysocki wrote:
> > > @@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state)
> > > break;
> > > }
> > > - return ret;
> > > + return ret < 0 ? ret : 0;
> > > }
> > > int cpufreq_boost_trigger_state(int state)
> >
> > IMO it is better to update the caller of this function to handle the
> > positive value possibly returned by it correctly.
>
> Could you elaborate why? Viresh seems to be ok with this solution.

And it is absolutely fine for Rafael to not agree with it :)

> As I see it the caller doesn't expect the positive value returned by the
> original freq_qos_update_request(). It just doesn't need to know whether the
> effective policy has been updated or not, it only needs to make sure the
> operations has been successful. Moreover the positive value is related only
> to the !last! active policy, which doesn't give the caller a full picture
> of the policy change anyway. So taking all of these into account I'd leave the
> fix as is.

Rafael: This function is called via a function pointer, which can call
this or a platform dependent routine (like in acpi-cpufreq.c), and it
would be reasonable IMO for the return of that callback to only look
for 0 or negative values, as is generally done in the kernel. And so
this solution looked okay to me as the positive return is coming from
the implementation detail here.

--
viresh

2020-05-18 09:55:21

by Wysocki, Rafael J

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On 5/18/2020 9:41 AM, Viresh Kumar wrote:
> On 16-05-20, 15:52, Serge Semin wrote:
>> On Fri, May 15, 2020 at 05:58:47PM +0200, Rafael J. Wysocki wrote:
>>>> @@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state)
>>>> break;
>>>> }
>>>> - return ret;
>>>> + return ret < 0 ? ret : 0;
>>>> }
>>>> int cpufreq_boost_trigger_state(int state)
>>> IMO it is better to update the caller of this function to handle the
>>> positive value possibly returned by it correctly.
>> Could you elaborate why? Viresh seems to be ok with this solution.
> And it is absolutely fine for Rafael to not agree with it :)
>
>> As I see it the caller doesn't expect the positive value returned by the
>> original freq_qos_update_request(). It just doesn't need to know whether the
>> effective policy has been updated or not, it only needs to make sure the
>> operations has been successful. Moreover the positive value is related only
>> to the !last! active policy, which doesn't give the caller a full picture
>> of the policy change anyway. So taking all of these into account I'd leave the
>> fix as is.
> Rafael: This function is called via a function pointer, which can call
> this or a platform dependent routine (like in acpi-cpufreq.c), and it
> would be reasonable IMO for the return of that callback to only look
> for 0 or negative values, as is generally done in the kernel.

But it only has one caller that can easily check ret < 0 instead of just
ret, so the extra branch can be saved.

That said if you really only want it to return 0 on success, you may as
well add a ret = 0; statement (with a comment explaining why it is
needed) after the last break in the loop.

Cheers!


2020-05-18 10:13:23

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> That said if you really only want it to return 0 on success, you may as well
> add a ret = 0; statement (with a comment explaining why it is needed) after
> the last break in the loop.

That can be done as well, but will be a bit less efficient as the loop
will execute once for each policy, and so the statement will run
multiple times. Though it isn't going to add any significant latency
in the code.

--
viresh

2020-05-18 10:27:22

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > That said if you really only want it to return 0 on success, you may as well
> > add a ret = 0; statement (with a comment explaining why it is needed) after
> > the last break in the loop.
>
> That can be done as well, but will be a bit less efficient as the loop
> will execute once for each policy, and so the statement will run
> multiple times. Though it isn't going to add any significant latency
> in the code.

Right.

However, the logic in this entire function looks somewhat less than
straightforward to me, because it looks like it should return an
error on the first policy without a frequency table (having a frequency
table depends on the driver and that is the same for all policies, so it
is pointless to iterate any further in that case).

Also, the error should not be -EINVAL, because that means "invalid
argument" which would be the state value.

So I would do something like this:

---
drivers/cpufreq/cpufreq.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
static int cpufreq_boost_set_sw(int state)
{
struct cpufreq_policy *policy;
- int ret = -EINVAL;

for_each_active_policy(policy) {
+ int ret;
+
if (!policy->freq_table)
- continue;
+ return -ENXIO;

ret = cpufreq_frequency_table_cpuinfo(policy,
policy->freq_table);
if (ret) {
pr_err("%s: Policy frequency update failed\n",
__func__);
- break;
+ return ret;
}

ret = freq_qos_update_request(policy->max_freq_req, policy->max);
if (ret < 0)
- break;
+ return ret;
}

- return ret;
+ return 0;
}

int cpufreq_boost_trigger_state(int state)



2020-05-18 10:28:19

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > That said if you really only want it to return 0 on success, you may as well
> > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > the last break in the loop.
> >
> > That can be done as well, but will be a bit less efficient as the loop
> > will execute once for each policy, and so the statement will run
> > multiple times. Though it isn't going to add any significant latency
> > in the code.
>
> Right.
>
> However, the logic in this entire function looks somewhat less than
> straightforward to me, because it looks like it should return an
> error on the first policy without a frequency table (having a frequency
> table depends on the driver and that is the same for all policies, so it
> is pointless to iterate any further in that case).
>
> Also, the error should not be -EINVAL, because that means "invalid
> argument" which would be the state value.
>
> So I would do something like this:
>
> ---
> drivers/cpufreq/cpufreq.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> static int cpufreq_boost_set_sw(int state)
> {
> struct cpufreq_policy *policy;
> - int ret = -EINVAL;
>
> for_each_active_policy(policy) {
> + int ret;
> +
> if (!policy->freq_table)
> - continue;
> + return -ENXIO;
>
> ret = cpufreq_frequency_table_cpuinfo(policy,
> policy->freq_table);
> if (ret) {
> pr_err("%s: Policy frequency update failed\n",
> __func__);
> - break;
> + return ret;
> }
>
> ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> if (ret < 0)
> - break;
> + return ret;
> }
>
> - return ret;
> + return 0;
> }
>
> int cpufreq_boost_trigger_state(int state)

Acked-by: Viresh Kumar <[email protected]>

--
viresh

2020-05-18 10:33:54

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
> On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> > On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > > That said if you really only want it to return 0 on success, you may as well
> > > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > > the last break in the loop.
> > >
> > > That can be done as well, but will be a bit less efficient as the loop
> > > will execute once for each policy, and so the statement will run
> > > multiple times. Though it isn't going to add any significant latency
> > > in the code.
> >
> > Right.
> >
> > However, the logic in this entire function looks somewhat less than
> > straightforward to me, because it looks like it should return an
> > error on the first policy without a frequency table (having a frequency
> > table depends on the driver and that is the same for all policies, so it
> > is pointless to iterate any further in that case).
> >
> > Also, the error should not be -EINVAL, because that means "invalid
> > argument" which would be the state value.
> >
> > So I would do something like this:
> >
> > ---
> > drivers/cpufreq/cpufreq.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> > static int cpufreq_boost_set_sw(int state)
> > {
> > struct cpufreq_policy *policy;
> > - int ret = -EINVAL;
> >
> > for_each_active_policy(policy) {
> > + int ret;
> > +
> > if (!policy->freq_table)
> > - continue;
> > + return -ENXIO;
> >
> > ret = cpufreq_frequency_table_cpuinfo(policy,
> > policy->freq_table);
> > if (ret) {
> > pr_err("%s: Policy frequency update failed\n",
> > __func__);
> > - break;
> > + return ret;
> > }
> >
> > ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> > if (ret < 0)
> > - break;
> > + return ret;
> > }
> >
> > - return ret;
> > + return 0;
> > }
> >
> > int cpufreq_boost_trigger_state(int state)
>
> Acked-by: Viresh Kumar <[email protected]>

Ok. Thanks for the comments. Shall I resend the patch with update Rafael
suggests or you'll merge the Rafael's fix in yourself?

-Sergey
>
> --
> viresh

2020-05-18 10:45:37

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
> On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
> > On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> > > On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > > > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > > > That said if you really only want it to return 0 on success, you may as well
> > > > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > > > the last break in the loop.
> > > >
> > > > That can be done as well, but will be a bit less efficient as the loop
> > > > will execute once for each policy, and so the statement will run
> > > > multiple times. Though it isn't going to add any significant latency
> > > > in the code.
> > >
> > > Right.
> > >
> > > However, the logic in this entire function looks somewhat less than
> > > straightforward to me, because it looks like it should return an
> > > error on the first policy without a frequency table (having a frequency
> > > table depends on the driver and that is the same for all policies, so it
> > > is pointless to iterate any further in that case).
> > >
> > > Also, the error should not be -EINVAL, because that means "invalid
> > > argument" which would be the state value.
> > >
> > > So I would do something like this:
> > >
> > > ---
> > > drivers/cpufreq/cpufreq.c | 11 ++++++-----
> > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > >
> > > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > > @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> > > static int cpufreq_boost_set_sw(int state)
> > > {
> > > struct cpufreq_policy *policy;
> > > - int ret = -EINVAL;
> > >
> > > for_each_active_policy(policy) {
> > > + int ret;
> > > +
> > > if (!policy->freq_table)
> > > - continue;
> > > + return -ENXIO;
> > >
> > > ret = cpufreq_frequency_table_cpuinfo(policy,
> > > policy->freq_table);
> > > if (ret) {
> > > pr_err("%s: Policy frequency update failed\n",
> > > __func__);
> > > - break;
> > > + return ret;
> > > }
> > >
> > > ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> > > if (ret < 0)
> > > - break;
> > > + return ret;
> > > }
> > >
> > > - return ret;
> > > + return 0;
> > > }
> > >
> > > int cpufreq_boost_trigger_state(int state)
> >
> > Acked-by: Viresh Kumar <[email protected]>
>
> Ok. Thanks for the comments. Shall I resend the patch with update Rafael
> suggests or you'll merge the Rafael's fix in yourself?

I'll apply the fix directly, thanks!



2020-05-18 10:50:38

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Mon, May 18, 2020 at 12:41:19PM +0200, Rafael J. Wysocki wrote:
> On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
> > On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
> > > On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> > > > On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > > > > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > > > > That said if you really only want it to return 0 on success, you may as well
> > > > > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > > > > the last break in the loop.
> > > > >
> > > > > That can be done as well, but will be a bit less efficient as the loop
> > > > > will execute once for each policy, and so the statement will run
> > > > > multiple times. Though it isn't going to add any significant latency
> > > > > in the code.
> > > >
> > > > Right.
> > > >
> > > > However, the logic in this entire function looks somewhat less than
> > > > straightforward to me, because it looks like it should return an
> > > > error on the first policy without a frequency table (having a frequency
> > > > table depends on the driver and that is the same for all policies, so it
> > > > is pointless to iterate any further in that case).
> > > >
> > > > Also, the error should not be -EINVAL, because that means "invalid
> > > > argument" which would be the state value.
> > > >
> > > > So I would do something like this:
> > > >
> > > > ---
> > > > drivers/cpufreq/cpufreq.c | 11 ++++++-----
> > > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > > >
> > > > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > > > ===================================================================
> > > > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > > > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > > > @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> > > > static int cpufreq_boost_set_sw(int state)
> > > > {
> > > > struct cpufreq_policy *policy;
> > > > - int ret = -EINVAL;
> > > >
> > > > for_each_active_policy(policy) {
> > > > + int ret;
> > > > +
> > > > if (!policy->freq_table)
> > > > - continue;
> > > > + return -ENXIO;
> > > >
> > > > ret = cpufreq_frequency_table_cpuinfo(policy,
> > > > policy->freq_table);
> > > > if (ret) {
> > > > pr_err("%s: Policy frequency update failed\n",
> > > > __func__);
> > > > - break;
> > > > + return ret;
> > > > }
> > > >
> > > > ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> > > > if (ret < 0)
> > > > - break;
> > > > + return ret;
> > > > }
> > > >
> > > > - return ret;
> > > > + return 0;
> > > > }
> > > >
> > > > int cpufreq_boost_trigger_state(int state)
> > >
> > > Acked-by: Viresh Kumar <[email protected]>
> >
> > Ok. Thanks for the comments. Shall I resend the patch with update Rafael
> > suggests or you'll merge the Rafael's fix in yourself?
>
> I'll apply the fix directly, thanks!

Great. Is it going to be available in the repo:
https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
?
I'll need it to back port into my local kernel tree. Thanks.

-Sergey

>
>
>

2020-05-18 10:53:14

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Mon, May 18, 2020 at 12:46 PM Serge Semin
<[email protected]> wrote:
>
> On Mon, May 18, 2020 at 12:41:19PM +0200, Rafael J. Wysocki wrote:
> > On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
> > > On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
> > > > On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> > > > > On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > > > > > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > > > > > That said if you really only want it to return 0 on success, you may as well
> > > > > > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > > > > > the last break in the loop.
> > > > > >
> > > > > > That can be done as well, but will be a bit less efficient as the loop
> > > > > > will execute once for each policy, and so the statement will run
> > > > > > multiple times. Though it isn't going to add any significant latency
> > > > > > in the code.
> > > > >
> > > > > Right.
> > > > >
> > > > > However, the logic in this entire function looks somewhat less than
> > > > > straightforward to me, because it looks like it should return an
> > > > > error on the first policy without a frequency table (having a frequency
> > > > > table depends on the driver and that is the same for all policies, so it
> > > > > is pointless to iterate any further in that case).
> > > > >
> > > > > Also, the error should not be -EINVAL, because that means "invalid
> > > > > argument" which would be the state value.
> > > > >
> > > > > So I would do something like this:
> > > > >
> > > > > ---
> > > > > drivers/cpufreq/cpufreq.c | 11 ++++++-----
> > > > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > > > >
> > > > > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > > > > ===================================================================
> > > > > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > > > > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > > > > @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> > > > > static int cpufreq_boost_set_sw(int state)
> > > > > {
> > > > > struct cpufreq_policy *policy;
> > > > > - int ret = -EINVAL;
> > > > >
> > > > > for_each_active_policy(policy) {
> > > > > + int ret;
> > > > > +
> > > > > if (!policy->freq_table)
> > > > > - continue;
> > > > > + return -ENXIO;
> > > > >
> > > > > ret = cpufreq_frequency_table_cpuinfo(policy,
> > > > > policy->freq_table);
> > > > > if (ret) {
> > > > > pr_err("%s: Policy frequency update failed\n",
> > > > > __func__);
> > > > > - break;
> > > > > + return ret;
> > > > > }
> > > > >
> > > > > ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> > > > > if (ret < 0)
> > > > > - break;
> > > > > + return ret;
> > > > > }
> > > > >
> > > > > - return ret;
> > > > > + return 0;
> > > > > }
> > > > >
> > > > > int cpufreq_boost_trigger_state(int state)
> > > >
> > > > Acked-by: Viresh Kumar <[email protected]>
> > >
> > > Ok. Thanks for the comments. Shall I resend the patch with update Rafael
> > > suggests or you'll merge the Rafael's fix in yourself?
> >
> > I'll apply the fix directly, thanks!
>
> Great. Is it going to be available in the repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
> ?

Yes, it is. Please see the bleeding-edge branch in there, thanks!

2020-05-18 10:59:37

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Mon, May 18, 2020 at 12:51:15PM +0200, Rafael J. Wysocki wrote:
> On Mon, May 18, 2020 at 12:46 PM Serge Semin
> <[email protected]> wrote:
> >
> > On Mon, May 18, 2020 at 12:41:19PM +0200, Rafael J. Wysocki wrote:
> > > On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
> > > > On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
> > > > > On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> > > > > > On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > > > > > > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > > > > > > That said if you really only want it to return 0 on success, you may as well
> > > > > > > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > > > > > > the last break in the loop.
> > > > > > >
> > > > > > > That can be done as well, but will be a bit less efficient as the loop
> > > > > > > will execute once for each policy, and so the statement will run
> > > > > > > multiple times. Though it isn't going to add any significant latency
> > > > > > > in the code.
> > > > > >
> > > > > > Right.
> > > > > >
> > > > > > However, the logic in this entire function looks somewhat less than
> > > > > > straightforward to me, because it looks like it should return an
> > > > > > error on the first policy without a frequency table (having a frequency
> > > > > > table depends on the driver and that is the same for all policies, so it
> > > > > > is pointless to iterate any further in that case).
> > > > > >
> > > > > > Also, the error should not be -EINVAL, because that means "invalid
> > > > > > argument" which would be the state value.
> > > > > >
> > > > > > So I would do something like this:
> > > > > >
> > > > > > ---
> > > > > > drivers/cpufreq/cpufreq.c | 11 ++++++-----
> > > > > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > > > > > ===================================================================
> > > > > > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > > > > > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > > > > > @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> > > > > > static int cpufreq_boost_set_sw(int state)
> > > > > > {
> > > > > > struct cpufreq_policy *policy;
> > > > > > - int ret = -EINVAL;
> > > > > >
> > > > > > for_each_active_policy(policy) {
> > > > > > + int ret;
> > > > > > +
> > > > > > if (!policy->freq_table)
> > > > > > - continue;
> > > > > > + return -ENXIO;
> > > > > >
> > > > > > ret = cpufreq_frequency_table_cpuinfo(policy,
> > > > > > policy->freq_table);
> > > > > > if (ret) {
> > > > > > pr_err("%s: Policy frequency update failed\n",
> > > > > > __func__);
> > > > > > - break;
> > > > > > + return ret;
> > > > > > }
> > > > > >
> > > > > > ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> > > > > > if (ret < 0)
> > > > > > - break;
> > > > > > + return ret;
> > > > > > }
> > > > > >
> > > > > > - return ret;
> > > > > > + return 0;
> > > > > > }
> > > > > >
> > > > > > int cpufreq_boost_trigger_state(int state)
> > > > >
> > > > > Acked-by: Viresh Kumar <[email protected]>
> > > >
> > > > Ok. Thanks for the comments. Shall I resend the patch with update Rafael
> > > > suggests or you'll merge the Rafael's fix in yourself?
> > >
> > > I'll apply the fix directly, thanks!
> >
> > Great. Is it going to be available in the repo:
> > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
> > ?
>
> Yes, it is. Please see the bleeding-edge branch in there, thanks!

No credits with at least Reported-by tag? That's sad.(

-Sergey

2020-05-18 11:10:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

On Mon, May 18, 2020 at 12:56 PM Serge Semin
<[email protected]> wrote:
>
> On Mon, May 18, 2020 at 12:51:15PM +0200, Rafael J. Wysocki wrote:
> > On Mon, May 18, 2020 at 12:46 PM Serge Semin
> > <[email protected]> wrote:
> > >
> > > On Mon, May 18, 2020 at 12:41:19PM +0200, Rafael J. Wysocki wrote:
> > > > On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
> > > > > On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
> > > > > > On 18-05-20, 12:22, Rafael J. Wysocki wrote:
> > > > > > > On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
> > > > > > > > On 18-05-20, 11:53, Rafael J. Wysocki wrote:
> > > > > > > > > That said if you really only want it to return 0 on success, you may as well
> > > > > > > > > add a ret = 0; statement (with a comment explaining why it is needed) after
> > > > > > > > > the last break in the loop.
> > > > > > > >
> > > > > > > > That can be done as well, but will be a bit less efficient as the loop
> > > > > > > > will execute once for each policy, and so the statement will run
> > > > > > > > multiple times. Though it isn't going to add any significant latency
> > > > > > > > in the code.
> > > > > > >
> > > > > > > Right.
> > > > > > >
> > > > > > > However, the logic in this entire function looks somewhat less than
> > > > > > > straightforward to me, because it looks like it should return an
> > > > > > > error on the first policy without a frequency table (having a frequency
> > > > > > > table depends on the driver and that is the same for all policies, so it
> > > > > > > is pointless to iterate any further in that case).
> > > > > > >
> > > > > > > Also, the error should not be -EINVAL, because that means "invalid
> > > > > > > argument" which would be the state value.
> > > > > > >
> > > > > > > So I would do something like this:
> > > > > > >
> > > > > > > ---
> > > > > > > drivers/cpufreq/cpufreq.c | 11 ++++++-----
> > > > > > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > > > > > >
> > > > > > > Index: linux-pm/drivers/cpufreq/cpufreq.c
> > > > > > > ===================================================================
> > > > > > > --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> > > > > > > +++ linux-pm/drivers/cpufreq/cpufreq.c
> > > > > > > @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
> > > > > > > static int cpufreq_boost_set_sw(int state)
> > > > > > > {
> > > > > > > struct cpufreq_policy *policy;
> > > > > > > - int ret = -EINVAL;
> > > > > > >
> > > > > > > for_each_active_policy(policy) {
> > > > > > > + int ret;
> > > > > > > +
> > > > > > > if (!policy->freq_table)
> > > > > > > - continue;
> > > > > > > + return -ENXIO;
> > > > > > >
> > > > > > > ret = cpufreq_frequency_table_cpuinfo(policy,
> > > > > > > policy->freq_table);
> > > > > > > if (ret) {
> > > > > > > pr_err("%s: Policy frequency update failed\n",
> > > > > > > __func__);
> > > > > > > - break;
> > > > > > > + return ret;
> > > > > > > }
> > > > > > >
> > > > > > > ret = freq_qos_update_request(policy->max_freq_req, policy->max);
> > > > > > > if (ret < 0)
> > > > > > > - break;
> > > > > > > + return ret;
> > > > > > > }
> > > > > > >
> > > > > > > - return ret;
> > > > > > > + return 0;
> > > > > > > }
> > > > > > >
> > > > > > > int cpufreq_boost_trigger_state(int state)
> > > > > >
> > > > > > Acked-by: Viresh Kumar <[email protected]>
> > > > >
> > > > > Ok. Thanks for the comments. Shall I resend the patch with update Rafael
> > > > > suggests or you'll merge the Rafael's fix in yourself?
> > > >
> > > > I'll apply the fix directly, thanks!
> > >
> > > Great. Is it going to be available in the repo:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
> > > ?
> >
> > Yes, it is. Please see the bleeding-edge branch in there, thanks!
>
> No credits with at least Reported-by tag? That's sad.(

OK, done now, but you are not the only reported of it, so I've added
the other reporter too.

Thanks!

2020-05-18 13:50:59

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > Thomas,
> > Could you take a look at my comment below so I could proceed with the
> > patchset v3 development?
>
> I can't help, but using r4k clocksource with changing frequency is
> probaly only usefull as a random generator. So IMHO the only two
> options are disabling it or implement what arch/x86/kernel/tsc.c does.
>
> Thomas.

Thomas, could you proceed with the rest of the patches review?
├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
└─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support

It would be great if I fixed more comments in the next patchset version.

-Sergey

>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-18 16:34:14

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > Thomas,
> > > Could you take a look at my comment below so I could proceed with the
> > > patchset v3 development?
> >
> > I can't help, but using r4k clocksource with changing frequency is
> > probaly only usefull as a random generator. So IMHO the only two
> > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> >
> > Thomas.
>
> Thomas, could you proceed with the rest of the patches review?
> ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support

both are not my call, but look ok to me.

> ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors

that's broken. A reg shift of 2 doesn't mean we could use 32bit access
to the registers on other platforms. As I don't think adding some ifdefery
makes things nicer, just implement the your prom_putchar in board code.

> ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support

looks ok so far.

> ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro

that is fine

> └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support

this is IMHO a dangerous change. Enabling write merging for any
CPU supporting it might triggers bugs. Do it in your board bringup
code and at the moment I don't see a reason for the rest of that
patch.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-18 21:00:14

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> > On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > > Thomas,
> > > > Could you take a look at my comment below so I could proceed with the
> > > > patchset v3 development?
> > >
> > > I can't help, but using r4k clocksource with changing frequency is
> > > probaly only usefull as a random generator. So IMHO the only two
> > > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> > >
> > > Thomas.
> >
> > Thomas, could you proceed with the rest of the patches review?
> > ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> > ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
>
> both are not my call, but look ok to me.

Can I add your Reviewed-by tag there then?

>
> > ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
>
> that's broken. A reg shift of 2 doesn't mean we could use 32bit access
> to the registers on other platforms. As I don't think adding some ifdefery
> makes things nicer, just implement the your prom_putchar in board code.

I thought about that initially, but then I decided to alter the generic
early_printk_8250 code instead. My version of prom_putchar() would be almost
the same as one implemented in the early_printk_8250 module except minor
modification of replacing readb/writeb methods with readl/writel. So I didn't
want to duplicate the code, but wanted to provide a general way to fix the
problem potentially also for another platforms.

Since you don't like this fix alternatively I'd suggest to add the reg_width
parameter passed to the setup_8250_early_printk_port() method like this:
-setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
- unsigned int timeout)
+setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
+ unsigned int reg_width, unsigned int timeout)

By reg_width parameter we could determine the actual width of the register:
static inline u8 serial_in(int offset)
{
- return readb(serial8250_base + (offset << serial8250_reg_shift));
+ u8 ret = 0xFF;
+
+ offset <<= serial8250_reg_shift;
+ switch (serial8250_reg_width) {
+ case 1:
+ ret = readb(serial8250_base + offset);
+ break;
+ case 2:
+ ret = readw(serial8250_base + offset);
+ break;
+ case 4:
+ ret = readl(serial8250_base + offset);
+ break;
+ default:
+ break;
+ }
+
+ return ret;
}

The similar modification will be implemented for serial_out(). I'll also modify
the currently available setup_8250_early_printk_port() calls so they would pass
the reg_with as 1 to activate the normal readb/writeb IO methods.

What do you think about this?

>
> > ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
>
> looks ok so far.

Can I add your Reviewed-by tag there then?

>
> > ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
>
> that is fine

Can I add your Reviewed-by tag there then?

>
> > └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
>
> this is IMHO a dangerous change. Enabling write merging for any
> CPU supporting it might triggers bugs. Do it in your board bringup
> code and at the moment I don't see a reason for the rest of that
> patch.

Let's at least leave the mm_config() implementation but without the write-merge
enabling by default. Providing features availability macro
cpu_has_mm_sysad/cpu_has_mm_full and c0 config fields
MIPS_CPU_MM_SYSAD/MIPS_CPU_MM_FULL defined will be useful in the platform-specific
Write-Merge enable/disable procedure. For instance, in the my prom_init() method
I could use them to implement a code pattern like:

+ if (cpu_has_mm_full) {
+ unsigned int config0 = read_c0_config();
+ config0 = (config0 & ~MIPS_CONF_MM) | MIPS_CONF_MM_FULL;
+ write_c0_config(config0);
+ }

By doing so I can manually enable/disable the MM feature in the
cpu-feature-overrides.h. Without that I'd have to locally define these macro,
which isn't good seeing they are in fact generic and can be useful for other
platforms with SYSAD and FULL MM feature available. What do you think?

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-19 01:55:20

by Xiongfeng Wang

[permalink] [raw]
Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting

Hi Rafael,

On 2020/5/18 19:05, Rafael J. Wysocki wrote:
> On Mon, May 18, 2020 at 12:56 PM Serge Semin
> <[email protected]> wrote:
>>
>> On Mon, May 18, 2020 at 12:51:15PM +0200, Rafael J. Wysocki wrote:
>>> On Mon, May 18, 2020 at 12:46 PM Serge Semin
>>> <[email protected]> wrote:
>>>>
>>>> On Mon, May 18, 2020 at 12:41:19PM +0200, Rafael J. Wysocki wrote:
>>>>> On Monday, May 18, 2020 12:31:02 PM CEST Serge Semin wrote:
>>>>>> On Mon, May 18, 2020 at 03:54:15PM +0530, Viresh Kumar wrote:
>>>>>>> On 18-05-20, 12:22, Rafael J. Wysocki wrote:
>>>>>>>> On Monday, May 18, 2020 12:11:09 PM CEST Viresh Kumar wrote:
>>>>>>>>> On 18-05-20, 11:53, Rafael J. Wysocki wrote:
>>>>>>>>>> That said if you really only want it to return 0 on success, you may as well
>>>>>>>>>> add a ret = 0; statement (with a comment explaining why it is needed) after
>>>>>>>>>> the last break in the loop.
>>>>>>>>>
>>>>>>>>> That can be done as well, but will be a bit less efficient as the loop
>>>>>>>>> will execute once for each policy, and so the statement will run
>>>>>>>>> multiple times. Though it isn't going to add any significant latency
>>>>>>>>> in the code.
>>>>>>>>
>>>>>>>> Right.
>>>>>>>>
>>>>>>>> However, the logic in this entire function looks somewhat less than
>>>>>>>> straightforward to me, because it looks like it should return an
>>>>>>>> error on the first policy without a frequency table (having a frequency
>>>>>>>> table depends on the driver and that is the same for all policies, so it
>>>>>>>> is pointless to iterate any further in that case).
>>>>>>>>
>>>>>>>> Also, the error should not be -EINVAL, because that means "invalid
>>>>>>>> argument" which would be the state value.
>>>>>>>>
>>>>>>>> So I would do something like this:
>>>>>>>>
>>>>>>>> ---
>>>>>>>> drivers/cpufreq/cpufreq.c | 11 ++++++-----
>>>>>>>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> Index: linux-pm/drivers/cpufreq/cpufreq.c
>>>>>>>> ===================================================================
>>>>>>>> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
>>>>>>>> +++ linux-pm/drivers/cpufreq/cpufreq.c
>>>>>>>> @@ -2535,26 +2535,27 @@ EXPORT_SYMBOL_GPL(cpufreq_update_limits)
>>>>>>>> static int cpufreq_boost_set_sw(int state)
>>>>>>>> {
>>>>>>>> struct cpufreq_policy *policy;
>>>>>>>> - int ret = -EINVAL;
>>>>>>>>
>>>>>>>> for_each_active_policy(policy) {
>>>>>>>> + int ret;
>>>>>>>> +
>>>>>>>> if (!policy->freq_table)
>>>>>>>> - continue;
>>>>>>>> + return -ENXIO;
>>>>>>>>
>>>>>>>> ret = cpufreq_frequency_table_cpuinfo(policy,
>>>>>>>> policy->freq_table);
>>>>>>>> if (ret) {
>>>>>>>> pr_err("%s: Policy frequency update failed\n",
>>>>>>>> __func__);
>>>>>>>> - break;
>>>>>>>> + return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> ret = freq_qos_update_request(policy->max_freq_req, policy->max);
>>>>>>>> if (ret < 0)
>>>>>>>> - break;
>>>>>>>> + return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> - return ret;
>>>>>>>> + return 0;
>>>>>>>> }
>>>>>>>>
>>>>>>>> int cpufreq_boost_trigger_state(int state)
>>>>>>>
>>>>>>> Acked-by: Viresh Kumar <[email protected]>
>>>>>>
>>>>>> Ok. Thanks for the comments. Shall I resend the patch with update Rafael
>>>>>> suggests or you'll merge the Rafael's fix in yourself?
>>>>>
>>>>> I'll apply the fix directly, thanks!
>>>>
>>>> Great. Is it going to be available in the repo:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/
>>>> ?
>>>
>>> Yes, it is. Please see the bleeding-edge branch in there, thanks!

Thanks for CCing me. I will write my next version based on this branch.

Thanks,
Xiongfeng

>>
>> No credits with at least Reported-by tag? That's sad.(
>
> OK, done now, but you are not the only reported of it, so I've added
> the other reporter too.
>
> Thanks!
>
> .
>

2020-05-19 16:02:05

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 12/20] mips: MAAR: Add XPA mode support

On Wed, May 06, 2020 at 08:42:30PM +0300, [email protected] wrote:
> From: Serge Semin <[email protected]>
>
> When XPA mode is enabled the normally 32-bits MAAR pair registers
> are extended to be of 64-bits width as in pure 64-bits MIPS
> architecture. In this case the MAAR registers can enable the
> speculative loads/stores for addresses of up to 39-bits width.
> But in this case the process of the MAAR initialization changes a bit.
> The upper 32-bits of the registers are supposed to be accessed by mean
> of the dedicated instructions mfhc0/mthc0 and there is a CP0.MAAR.VH
> bit which should be set together with CP0.MAAR.VL as indication
> of the boundary validity. All of these peculiarities were taken into
> account in this commit so the speculative loads/stores would work
> when XPA mode is enabled.
>
> Co-developed-by: Alexey Malahov <[email protected]>
> Signed-off-by: Alexey Malahov <[email protected]>
> Signed-off-by: Serge Semin <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Paul Burton <[email protected]>
> Cc: Ralf Baechle <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/mips/include/asm/maar.h | 17 +++++++++++++++--
> arch/mips/include/asm/mipsregs.h | 10 ++++++++++
> arch/mips/mm/init.c | 8 +++++++-
> 3 files changed, 32 insertions(+), 3 deletions(-)

applied to mips-next.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-19 16:02:17

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Mon, May 18, 2020 at 11:57:52PM +0300, Serge Semin wrote:
> On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> > On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> > > On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > > > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > > > Thomas,
> > > > > Could you take a look at my comment below so I could proceed with the
> > > > > patchset v3 development?
> > > >
> > > > I can't help, but using r4k clocksource with changing frequency is
> > > > probaly only usefull as a random generator. So IMHO the only two
> > > > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> > > >
> > > > Thomas.
> > >
> > > Thomas, could you proceed with the rest of the patches review?
> > > ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> > > ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
> >
> > both are not my call, but look ok to me.
>
> Can I add your Reviewed-by tag there then?

only for 16/20. 15/20 looks ok to me, but I have not enough insides
on the hardware to say this is good.

> > > ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
> >
> > that's broken. A reg shift of 2 doesn't mean we could use 32bit access
> > to the registers on other platforms. As I don't think adding some ifdefery
> > makes things nicer, just implement the your prom_putchar in board code.
>
> I thought about that initially, but then I decided to alter the generic
> early_printk_8250 code instead. My version of prom_putchar() would be almost
> the same as one implemented in the early_printk_8250 module except minor
> modification of replacing readb/writeb methods with readl/writel. So I didn't
> want to duplicate the code, but wanted to provide a general way to fix the
> problem potentially also for another platforms.
>
> Since you don't like this fix alternatively I'd suggest to add the reg_width
> parameter passed to the setup_8250_early_printk_port() method like this:
> -setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> - unsigned int timeout)
> +setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> + unsigned int reg_width, unsigned int timeout)
>
> By reg_width parameter we could determine the actual width of the register:
> static inline u8 serial_in(int offset)
> {
> - return readb(serial8250_base + (offset << serial8250_reg_shift));
> + u8 ret = 0xFF;
> +
> + offset <<= serial8250_reg_shift;
> + switch (serial8250_reg_width) {
> + case 1:
> + ret = readb(serial8250_base + offset);
> + break;
> + case 2:
> + ret = readw(serial8250_base + offset);
> + break;
> + case 4:
> + ret = readl(serial8250_base + offset);
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> }
>
> The similar modification will be implemented for serial_out(). I'll also modify

look at the lines of code you are adding. Doing your own prom_putchar will
probably have less lines.

> What do you think about this?

please do your own prom_putchar.


> >
> > > ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
> >
> > looks ok so far.
>
> Can I add your Reviewed-by tag there then?

As I'm the maintainer of the part, I've simply applied it.

> >
> > > ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
> >
> > that is fine
>
> Can I add your Reviewed-by tag there then?

As this didn't apply cleanly, I'll apply it after you've resent it.
IMHO no need for a Reviewed-by.

> > > └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
> >
> > this is IMHO a dangerous change. Enabling write merging for any
> > CPU supporting it might triggers bugs. Do it in your board bringup
> > code and at the moment I don't see a reason for the rest of that
> > patch.
>
> Let's at least leave the mm_config() implementation but without the write-merge
> enabling by default. Providing features availability macro
> cpu_has_mm_sysad/cpu_has_mm_full and c0 config fields

do you have a user of that ? I'm not introducing code nobody uses.

> I could use them to implement a code pattern like:
>
> + if (cpu_has_mm_full) {
> + unsigned int config0 = read_c0_config();
> + config0 = (config0 & ~MIPS_CONF_MM) | MIPS_CONF_MM_FULL;
> + write_c0_config(config0);
> + }

you know you are running on a R5 core, so you know you have MM_FULL.
No need to check this.

> By doing so I can manually enable/disable the MM feature in the
> cpu-feature-overrides.h. Without that I'd have to locally define these macro,
> which isn't good seeing they are in fact generic and can be useful for other
> platforms with SYSAD and FULL MM feature available. What do you think?

To me this is a hardware feature I expect to be done by firmware and
Linux shouldn't care about it, if it doesn't have any software
implications.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-20 11:33:15

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 12/20] mips: MAAR: Add XPA mode support

On Tue, May 19, 2020 at 05:42:13PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 06, 2020 at 08:42:30PM +0300, [email protected] wrote:
> > From: Serge Semin <[email protected]>
> >
> > When XPA mode is enabled the normally 32-bits MAAR pair registers
> > are extended to be of 64-bits width as in pure 64-bits MIPS
> > architecture. In this case the MAAR registers can enable the
> > speculative loads/stores for addresses of up to 39-bits width.
> > But in this case the process of the MAAR initialization changes a bit.
> > The upper 32-bits of the registers are supposed to be accessed by mean
> > of the dedicated instructions mfhc0/mthc0 and there is a CP0.MAAR.VH
> > bit which should be set together with CP0.MAAR.VL as indication
> > of the boundary validity. All of these peculiarities were taken into
> > account in this commit so the speculative loads/stores would work
> > when XPA mode is enabled.
> >
> > Co-developed-by: Alexey Malahov <[email protected]>
> > Signed-off-by: Alexey Malahov <[email protected]>
> > Signed-off-by: Serge Semin <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Paul Burton <[email protected]>
> > Cc: Ralf Baechle <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > arch/mips/include/asm/maar.h | 17 +++++++++++++++--
> > arch/mips/include/asm/mipsregs.h | 10 ++++++++++
> > arch/mips/mm/init.c | 8 +++++++-
> > 3 files changed, 32 insertions(+), 3 deletions(-)
>
> applied to mips-next.

Great! Thanks.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-20 12:03:48

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Tue, May 19, 2020 at 05:50:53PM +0200, Thomas Bogendoerfer wrote:
> On Mon, May 18, 2020 at 11:57:52PM +0300, Serge Semin wrote:
> > On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> > > On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> > > > On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > > > > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > > > > Thomas,
> > > > > > Could you take a look at my comment below so I could proceed with the
> > > > > > patchset v3 development?
> > > > >
> > > > > I can't help, but using r4k clocksource with changing frequency is
> > > > > probaly only usefull as a random generator. So IMHO the only two
> > > > > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> > > > >
> > > > > Thomas.
> > > >
> > > > Thomas, could you proceed with the rest of the patches review?
> > > > ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> > > > ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
> > >
> > > both are not my call, but look ok to me.
> >
> > Can I add your Reviewed-by tag there then?
>
> only for 16/20. 15/20 looks ok to me, but I have not enough insides
> on the hardware to say this is good.

Ok.

>
> > > > ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
> > >
> > > that's broken. A reg shift of 2 doesn't mean we could use 32bit access
> > > to the registers on other platforms. As I don't think adding some ifdefery
> > > makes things nicer, just implement the your prom_putchar in board code.
> >
> > I thought about that initially, but then I decided to alter the generic
> > early_printk_8250 code instead. My version of prom_putchar() would be almost
> > the same as one implemented in the early_printk_8250 module except minor
> > modification of replacing readb/writeb methods with readl/writel. So I didn't
> > want to duplicate the code, but wanted to provide a general way to fix the
> > problem potentially also for another platforms.
> >
> > Since you don't like this fix alternatively I'd suggest to add the reg_width
> > parameter passed to the setup_8250_early_printk_port() method like this:
> > -setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> > - unsigned int timeout)
> > +setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> > + unsigned int reg_width, unsigned int timeout)
> >
> > By reg_width parameter we could determine the actual width of the register:
> > static inline u8 serial_in(int offset)
> > {
> > - return readb(serial8250_base + (offset << serial8250_reg_shift));
> > + u8 ret = 0xFF;
> > +
> > + offset <<= serial8250_reg_shift;
> > + switch (serial8250_reg_width) {
> > + case 1:
> > + ret = readb(serial8250_base + offset);
> > + break;
> > + case 2:
> > + ret = readw(serial8250_base + offset);
> > + break;
> > + case 4:
> > + ret = readl(serial8250_base + offset);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return ret;
> > }
> >
> > The similar modification will be implemented for serial_out(). I'll also modify
>
> look at the lines of code you are adding. Doing your own prom_putchar will
> probably have less lines.
>
> > What do you think about this?
>
> please do your own prom_putchar.
>

Ok.

>
> > >
> > > > ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
> > >
> > > looks ok so far.
> >
> > Can I add your Reviewed-by tag there then?
>
> As I'm the maintainer of the part, I've simply applied it.
>
> > >
> > > > ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
> > >
> > > that is fine
> >
> > Can I add your Reviewed-by tag there then?
>
> As this didn't apply cleanly, I'll apply it after you've resent it.
> IMHO no need for a Reviewed-by.

Ok.

>
> > > > └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
> > >
> > > this is IMHO a dangerous change. Enabling write merging for any
> > > CPU supporting it might triggers bugs. Do it in your board bringup
> > > code and at the moment I don't see a reason for the rest of that
> > > patch.
> >
> > Let's at least leave the mm_config() implementation but without the write-merge
> > enabling by default. Providing features availability macro
> > cpu_has_mm_sysad/cpu_has_mm_full and c0 config fields
>
> do you have a user of that ? I'm not introducing code nobody uses.
>

See my comment below.

> > I could use them to implement a code pattern like:
> >
> > + if (cpu_has_mm_full) {
> > + unsigned int config0 = read_c0_config();
> > + config0 = (config0 & ~MIPS_CONF_MM) | MIPS_CONF_MM_FULL;
> > + write_c0_config(config0);
> > + }
>
> you know you are running on a R5 core, so you know you have MM_FULL.
> No need to check this.
>
> > By doing so I can manually enable/disable the MM feature in the
> > cpu-feature-overrides.h. Without that I'd have to locally define these macro,
> > which isn't good seeing they are in fact generic and can be useful for other
> > platforms with SYSAD and FULL MM feature available. What do you think?
>
> To me this is a hardware feature I expect to be done by firmware and
> Linux shouldn't care about it, if it doesn't have any software
> implications.

I think there is a misunderstanding here. In this patch I am not enabling
Write-Merge feature for any memory range. I am enabling the UCA Cache Coherency
attribute to be available for utilization. See the user-manual info regarding
the CP0.CONFIG.MM field:
Write Merge.This bit indicates whether write-through merging is enabled
in the 32-byte collapsing write buffer.
0: No merging allowed
1: Merging allowed

In order to have the Write-merging really enabled for a particular PFN one have
to mark its TLB entry with UCA (EntryLoX.C[3:5] = 7) attribute. So in this patch
I am attempting to detect whether the feature is either already enabled or if
available to enable it for utilization.

If there is no misunderstanding and you said what you said, that even enabling
the feature for utilization might be dangerous, let's at least leave the
MIPS_CONF_MM, MIPS_CONF_MM_FULL and MIPS_CONF_MM_SYS_SYSAD fields
definition in the "arch/mips/include/asm/mipsregs.h" header. I'll use
them to enable the write-merge in my platform code.

What do you think?

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-20 12:16:39

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Tue, May 19, 2020 at 05:50:53PM +0200, Thomas Bogendoerfer wrote:
> On Mon, May 18, 2020 at 11:57:52PM +0300, Serge Semin wrote:
> > On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> > > On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> > > > On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > > > > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > > > > Thomas,
> > > > > > Could you take a look at my comment below so I could proceed with the
> > > > > > patchset v3 development?
> > > > >
> > > > > I can't help, but using r4k clocksource with changing frequency is
> > > > > probaly only usefull as a random generator. So IMHO the only two
> > > > > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> > > > >
> > > > > Thomas.
> > > >
> > > > Thomas, could you proceed with the rest of the patches review?
> > > > ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> > > > ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
> > >
> > > both are not my call, but look ok to me.
> >
> > Can I add your Reviewed-by tag there then?
>
> only for 16/20. 15/20 looks ok to me, but I have not enough insides
> on the hardware to say this is good.
>
> > > > ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
> > >
> > > that's broken. A reg shift of 2 doesn't mean we could use 32bit access
> > > to the registers on other platforms. As I don't think adding some ifdefery
> > > makes things nicer, just implement the your prom_putchar in board code.
> >
> > I thought about that initially, but then I decided to alter the generic
> > early_printk_8250 code instead. My version of prom_putchar() would be almost
> > the same as one implemented in the early_printk_8250 module except minor
> > modification of replacing readb/writeb methods with readl/writel. So I didn't
> > want to duplicate the code, but wanted to provide a general way to fix the
> > problem potentially also for another platforms.
> >
> > Since you don't like this fix alternatively I'd suggest to add the reg_width
> > parameter passed to the setup_8250_early_printk_port() method like this:
> > -setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> > - unsigned int timeout)
> > +setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> > + unsigned int reg_width, unsigned int timeout)
> >
> > By reg_width parameter we could determine the actual width of the register:
> > static inline u8 serial_in(int offset)
> > {
> > - return readb(serial8250_base + (offset << serial8250_reg_shift));
> > + u8 ret = 0xFF;
> > +
> > + offset <<= serial8250_reg_shift;
> > + switch (serial8250_reg_width) {
> > + case 1:
> > + ret = readb(serial8250_base + offset);
> > + break;
> > + case 2:
> > + ret = readw(serial8250_base + offset);
> > + break;
> > + case 4:
> > + ret = readl(serial8250_base + offset);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return ret;
> > }
> >
> > The similar modification will be implemented for serial_out(). I'll also modify
>
> look at the lines of code you are adding. Doing your own prom_putchar will
> probably have less lines.
>
> > What do you think about this?
>
> please do your own prom_putchar.

One more time regarding this problem but in appliance to another part of the
MIPS code. I've missed the patch to draw your attention to:
[PATCH v2 14/20] mips: Use offset-sized IO-mem accessors in CPS debug printout

There I've applied the same fix as in the patch:
[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors

Since you don't like the way I initially fixed it, suppose there we don't have
another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
parameter to select a proper accessors, like sw in our case, and sb by defaul).
Right?

(Note UART_L is incorrectly created in that patch, I'll remove that macro in
v3.)

-Sergey

>
>
> > >
> > > > ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
> > >
> > > looks ok so far.
> >
> > Can I add your Reviewed-by tag there then?
>
> As I'm the maintainer of the part, I've simply applied it.
>
> > >
> > > > ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
> > >
> > > that is fine
> >
> > Can I add your Reviewed-by tag there then?
>
> As this didn't apply cleanly, I'll apply it after you've resent it.
> IMHO no need for a Reviewed-by.
>
> > > > └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
> > >
> > > this is IMHO a dangerous change. Enabling write merging for any
> > > CPU supporting it might triggers bugs. Do it in your board bringup
> > > code and at the moment I don't see a reason for the rest of that
> > > patch.
> >
> > Let's at least leave the mm_config() implementation but without the write-merge
> > enabling by default. Providing features availability macro
> > cpu_has_mm_sysad/cpu_has_mm_full and c0 config fields
>
> do you have a user of that ? I'm not introducing code nobody uses.
>
> > I could use them to implement a code pattern like:
> >
> > + if (cpu_has_mm_full) {
> > + unsigned int config0 = read_c0_config();
> > + config0 = (config0 & ~MIPS_CONF_MM) | MIPS_CONF_MM_FULL;
> > + write_c0_config(config0);
> > + }
>
> you know you are running on a R5 core, so you know you have MM_FULL.
> No need to check this.
>
> > By doing so I can manually enable/disable the MM feature in the
> > cpu-feature-overrides.h. Without that I'd have to locally define these macro,
> > which isn't good seeing they are in fact generic and can be useful for other
> > platforms with SYSAD and FULL MM feature available. What do you think?
>
> To me this is a hardware feature I expect to be done by firmware and
> Linux shouldn't care about it, if it doesn't have any software
> implications.
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-20 12:24:03

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 03:12:02PM +0300, Serge Semin wrote:
> On Tue, May 19, 2020 at 05:50:53PM +0200, Thomas Bogendoerfer wrote:
> > On Mon, May 18, 2020 at 11:57:52PM +0300, Serge Semin wrote:
> > > On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> > > > On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:

[nip]

> > > > > ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
> > > >
> > > > that's broken. A reg shift of 2 doesn't mean we could use 32bit access
> > > > to the registers on other platforms. As I don't think adding some ifdefery
> > > > makes things nicer, just implement the your prom_putchar in board code.
> > >
> > > I thought about that initially, but then I decided to alter the generic
> > > early_printk_8250 code instead. My version of prom_putchar() would be almost
> > > the same as one implemented in the early_printk_8250 module except minor
> > > modification of replacing readb/writeb methods with readl/writel. So I didn't
> > > want to duplicate the code, but wanted to provide a general way to fix the
> > > problem potentially also for another platforms.
> > >
> > > Since you don't like this fix alternatively I'd suggest to add the reg_width
> > > parameter passed to the setup_8250_early_printk_port() method like this:
> > > -setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> > > - unsigned int timeout)
> > > +setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> > > + unsigned int reg_width, unsigned int timeout)
> > >
> > > By reg_width parameter we could determine the actual width of the register:
> > > static inline u8 serial_in(int offset)
> > > {
> > > - return readb(serial8250_base + (offset << serial8250_reg_shift));
> > > + u8 ret = 0xFF;
> > > +
> > > + offset <<= serial8250_reg_shift;
> > > + switch (serial8250_reg_width) {
> > > + case 1:
> > > + ret = readb(serial8250_base + offset);
> > > + break;
> > > + case 2:
> > > + ret = readw(serial8250_base + offset);
> > > + break;
> > > + case 4:
> > > + ret = readl(serial8250_base + offset);
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > +
> > > + return ret;
> > > }
> > >
> > > The similar modification will be implemented for serial_out(). I'll also modify
> >
> > look at the lines of code you are adding. Doing your own prom_putchar will
> > probably have less lines.
> >
> > > What do you think about this?
> >
> > please do your own prom_putchar.
>
> One more time regarding this problem but in appliance to another part of the
> MIPS code. I've missed the patch to draw your attention to:
> [PATCH v2 14/20] mips: Use offset-sized IO-mem accessors in CPS debug printout
>
> There I've applied the same fix as in the patch:
> [PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
>
> Since you don't like the way I initially fixed it, suppose there we don't have
> another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
> parameter to select a proper accessors, like sw in our case, and sb by defaul).
> Right?
>

> (Note UART_L is incorrectly created in that patch, I'll remove that macro in
> v3.)

Hm, actually no. The macro is correct. According to the code _mips_cps_putc()
always perform lw from the UART MMIO registers. This must be a bug. Don't you
think?

-Sergey

>
> -Sergey
>

2020-05-20 13:41:04

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 03:12:01PM +0300, Serge Semin wrote:
> Since you don't like the way I initially fixed it, suppose there we don't have
> another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
> parameter to select a proper accessors, like sw in our case, and sb by defaul).
> Right?

to be on the safe side it's probably the best thing. But I don't know
enough about CPS_NS16550 to judge whether shift value correlates with
possible access width.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-20 13:50:49

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 03:38:27PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 20, 2020 at 03:12:01PM +0300, Serge Semin wrote:
> > Since you don't like the way I initially fixed it, suppose there we don't have
> > another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
> > parameter to select a proper accessors, like sw in our case, and sb by defaul).
> > Right?
>
> to be on the safe side it's probably the best thing. But I don't know
> enough about CPS_NS16550 to judge whether shift value correlates with
> possible access width.

The base address passed to the _mips_cps_putc() leaf is UART-base address. It
has nothing to do with CPS. See:
/**
* _mips_cps_putc() - write a character to the UART
* @a0: ASCII character to write
* @t9: UART base address
*/
LEAF(_mips_cps_putc)
1: lw t0, UART_LSR_OFS(t9)
andi t0, t0, UART_LSR_TEMT
beqz t0, 1b
sb a0, UART_TX_OFS(t9)
jr ra
END(_mips_cps_putc)

So it's base address must be accessed with proper alignment. On our case it's
lw/sw instructions. Regarding using lw in the first line of the function. That's
must be a bug, since further in the same function they use sb to access the UART
Tx register. So reading a data from UART_LSR register should be also byte-sized
by using lb.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-20 14:05:30

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 02:59:27PM +0300, Serge Semin wrote:
> On Tue, May 19, 2020 at 05:50:53PM +0200, Thomas Bogendoerfer wrote:
> > On Mon, May 18, 2020 at 11:57:52PM +0300, Serge Semin wrote:
> > > On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> > > > On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> > > > > On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:

[nip]

> > > > > └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
> > > >
> > > > this is IMHO a dangerous change. Enabling write merging for any
> > > > CPU supporting it might triggers bugs. Do it in your board bringup
> > > > code and at the moment I don't see a reason for the rest of that
> > > > patch.
> > >
> > > Let's at least leave the mm_config() implementation but without the write-merge
> > > enabling by default. Providing features availability macro
> > > cpu_has_mm_sysad/cpu_has_mm_full and c0 config fields
> >
> > do you have a user of that ? I'm not introducing code nobody uses.
> >
>
> See my comment below.
>
> > > I could use them to implement a code pattern like:
> > >
> > > + if (cpu_has_mm_full) {
> > > + unsigned int config0 = read_c0_config();
> > > + config0 = (config0 & ~MIPS_CONF_MM) | MIPS_CONF_MM_FULL;
> > > + write_c0_config(config0);
> > > + }
> >
> > you know you are running on a R5 core, so you know you have MM_FULL.
> > No need to check this.
> >
> > > By doing so I can manually enable/disable the MM feature in the
> > > cpu-feature-overrides.h. Without that I'd have to locally define these macro,
> > > which isn't good seeing they are in fact generic and can be useful for other
> > > platforms with SYSAD and FULL MM feature available. What do you think?
> >
> > To me this is a hardware feature I expect to be done by firmware and
> > Linux shouldn't care about it, if it doesn't have any software
> > implications.
>
> I think there is a misunderstanding here. In this patch I am not enabling
> Write-Merge feature for any memory range. I am enabling the UCA Cache Coherency
> attribute to be available for utilization. See the user-manual info regarding
> the CP0.CONFIG.MM field:
> Write Merge.This bit indicates whether write-through merging is enabled
> in the 32-byte collapsing write buffer.
> 0: No merging allowed
> 1: Merging allowed
>
> In order to have the Write-merging really enabled for a particular PFN one have
> to mark its TLB entry with UCA (EntryLoX.C[3:5] = 7) attribute. So in this patch
> I am attempting to detect whether the feature is either already enabled or if
> available to enable it for utilization.
>
> If there is no misunderstanding and you said what you said, that even enabling
> the feature for utilization might be dangerous, let's at least leave the
> MIPS_CONF_MM, MIPS_CONF_MM_FULL and MIPS_CONF_MM_SYS_SYSAD fields
> definition in the "arch/mips/include/asm/mipsregs.h" header. I'll use
> them to enable the write-merge in my platform code.
>
> What do you think?
>

Thomas,
Could you also give me your comment on the above, so to make sure that we
understood each other correctly in this question?

-Sergey

2020-05-20 18:42:22

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 02:59:26PM +0300, Serge Semin wrote:
> I think there is a misunderstanding here. In this patch I am not enabling

you are right, I've missed the fact, that this also needs to be enabled
in TLB entries. Strange that MIPS added the enable bit while R10k simply
do uncached acclerated, whenever TLB entry selects it.

> If there is no misunderstanding and you said what you said, that even enabling
> the feature for utilization might be dangerous, let's at least leave the
> MIPS_CONF_MM, MIPS_CONF_MM_FULL and MIPS_CONF_MM_SYS_SYSAD fields
> definition in the "arch/mips/include/asm/mipsregs.h" header. I'll use
> them to enable the write-merge in my platform code.
>
> What do you think?

I withdraw my concerns and will apply the patch as is.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-20 18:42:34

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 04:48:26PM +0300, Serge Semin wrote:
> On Wed, May 20, 2020 at 03:38:27PM +0200, Thomas Bogendoerfer wrote:
> > On Wed, May 20, 2020 at 03:12:01PM +0300, Serge Semin wrote:
> > > Since you don't like the way I initially fixed it, suppose there we don't have
> > > another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
> > > parameter to select a proper accessors, like sw in our case, and sb by defaul).
> > > Right?
> >
> > to be on the safe side it's probably the best thing. But I don't know
> > enough about CPS_NS16550 to judge whether shift value correlates with
> > possible access width.
>
> The base address passed to the _mips_cps_putc() leaf is UART-base address. It
> has nothing to do with CPS. See:

ok, I'm confused. So this isn't an uart inside CPS hardware, but an uart used
by CPS code for debug output, right ?

To solve the issued please add CONFIG_MIPS_CPS_NS16550_WIDTH to select the
correct access width.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]

2020-05-20 21:13:54

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 08:30:57PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 20, 2020 at 04:48:26PM +0300, Serge Semin wrote:
> > On Wed, May 20, 2020 at 03:38:27PM +0200, Thomas Bogendoerfer wrote:
> > > On Wed, May 20, 2020 at 03:12:01PM +0300, Serge Semin wrote:
> > > > Since you don't like the way I initially fixed it, suppose there we don't have
> > > > another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
> > > > parameter to select a proper accessors, like sw in our case, and sb by defaul).
> > > > Right?
> > >
> > > to be on the safe side it's probably the best thing. But I don't know
> > > enough about CPS_NS16550 to judge whether shift value correlates with
> > > possible access width.
> >
> > The base address passed to the _mips_cps_putc() leaf is UART-base address. It
> > has nothing to do with CPS. See:
>
> ok, I'm confused. So this isn't an uart inside CPS hardware, but an uart used
> by CPS code for debug output, right ?

Right. It's not CPS, but just UART available on the system. See a comment in the
arch/mips/kernel/cps-vec-ns16550.S:
/**
* mips_cps_bev_dump() - dump relevant exception state to UART
* @a0: pointer to NULL-terminated ASCII string naming the exception
*
* Write information that may be useful in debugging an exception to the
* UART configured by CONFIG_MIPS_CPS_NS16550_*.
*...
*/
LEAF(mips_cps_bev_dump)
move s0, ra
move s1, a0

li t9, CKSEG1ADDR(CONFIG_MIPS_CPS_NS16550_BASE)
...

See the base is just loaded to the t9 register.

>
> To solve the issued please add CONFIG_MIPS_CPS_NS16550_WIDTH to select the
> correct access width.

Ok. Thanks.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]

2020-05-20 21:14:54

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled

On Wed, May 20, 2020 at 08:40:24PM +0200, Thomas Bogendoerfer wrote:
> On Wed, May 20, 2020 at 02:59:26PM +0300, Serge Semin wrote:
> > I think there is a misunderstanding here. In this patch I am not enabling
>
> you are right, I've missed the fact, that this also needs to be enabled
> in TLB entries. Strange that MIPS added the enable bit while R10k simply
> do uncached acclerated, whenever TLB entry selects it.
>
> > If there is no misunderstanding and you said what you said, that even enabling
> > the feature for utilization might be dangerous, let's at least leave the
> > MIPS_CONF_MM, MIPS_CONF_MM_FULL and MIPS_CONF_MM_SYS_SYSAD fields
> > definition in the "arch/mips/include/asm/mipsregs.h" header. I'll use
> > them to enable the write-merge in my platform code.
> >
> > What do you think?
>
> I withdraw my concerns and will apply the patch as is.

Great! Thanks.

-Sergey

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]