Re: [PATCH] cpufreq: Covert to exit callback returning void

2024-04-10 Thread Florian Fainelli



On 4/10/2024 7:33 AM, lizhe wrote:

Hi,
      i have already change the definition of exit
      in struct cpu_freq_driver in include/linux/cpufreq.h


Again, no top-posting and please use HTML. The patch you sent does not 
indicate that include/linux/cpufreq.h has been updated, so maybe you 
forgot to "git add include/linux/cpufreq.h" before preparing the patch.


You can verify the list of files changed in your patch by looking at the 
lines after the "---":


---
 drivers/cpufreq/acpi-cpufreq.c | 4 +---
 drivers/cpufreq/amd-pstate.c   | 7 ++-
 drivers/cpufreq/apple-soc-cpufreq.c| 4 +---
 drivers/cpufreq/bmips-cpufreq.c| 4 +---
 drivers/cpufreq/cppc_cpufreq.c | 3 +--
 drivers/cpufreq/cpufreq-dt.c   | 3 +--
 drivers/cpufreq/e_powersaver.c | 3 +--
 drivers/cpufreq/intel_pstate.c | 4 +---
 drivers/cpufreq/mediatek-cpufreq-hw.c  | 4 +---
 drivers/cpufreq/mediatek-cpufreq.c | 4 +---
 drivers/cpufreq/omap-cpufreq.c | 3 +--
 drivers/cpufreq/pasemi-cpufreq.c   | 6 ++
 drivers/cpufreq/powernow-k6.c  | 3 +--
 drivers/cpufreq/powernow-k7.c  | 3 +--
 drivers/cpufreq/powernow-k8.c  | 4 +---
 drivers/cpufreq/powernv-cpufreq.c  | 4 +---
 drivers/cpufreq/ppc_cbe_cpufreq.c  | 3 +--
 drivers/cpufreq/qcom-cpufreq-hw.c  | 4 +---
 drivers/cpufreq/qoriq-cpufreq.c| 4 +---
 drivers/cpufreq/scmi-cpufreq.c | 4 +---
 drivers/cpufreq/scpi-cpufreq.c | 4 +---
 drivers/cpufreq/sh-cpufreq.c   | 4 +---
 drivers/cpufreq/sparc-us2e-cpufreq.c   | 3 +--
 drivers/cpufreq/sparc-us3-cpufreq.c| 3 +--
 drivers/cpufreq/speedstep-centrino.c   | 4 +---
 drivers/cpufreq/tegra194-cpufreq.c | 4 +---
 drivers/cpufreq/vexpress-spc-cpufreq.c | 3 +--
 27 files changed, 29 insertions(+), 74 deletions(-)
--
Florian


smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH 1/4] arm: ptdump: Rename CONFIG_DEBUG_WX to CONFIG_ARM_DEBUG_WX

2024-01-09 Thread Florian Fainelli

On 1/9/24 04:14, Christophe Leroy wrote:

CONFIG_DEBUG_WX is a core option defined in mm/Kconfig.debug

To avoid any future conflict, rename ARM version
into CONFIG_ARM_DEBUG_WX.

Signed-off-by: Christophe Leroy 


Looks fine, you might also want to 
s/CONFIG_DEBUG_WX/CONFIG_ARM_DEBUG_WX/ in 
arch/arm/configs/aspeed_g{4,5}_defconfig so there are no surprises when 
people pull in those changes.

--
Florian



Re: [PATCH 11/22] mips/cpu: Mark play_dead() __noreturn

2023-02-06 Thread Florian Fainelli

On 2/3/23 14:05, Josh Poimboeuf wrote:

play_dead() doesn't return.  Annotate it as such.  By extension this
also makes arch_cpu_idle_dead() noreturn.

Signed-off-by: Josh Poimboeuf 


Acked-by: Florian Fainelli 
--
Florian



Re: [PATCH 10/22] mips/cpu: Make sure play_dead() doesn't return

2023-02-06 Thread Florian Fainelli

On 2/3/23 14:05, Josh Poimboeuf wrote:

play_dead() doesn't return.  Make that more explicit with a BUG().

BUG() is preferable to unreachable() because BUG() is a more explicit
failure mode and avoids undefined behavior like falling off the edge of
the function into whatever code happens to be next.

Signed-off-by: Josh Poimboeuf 


Acked-by: Florian Fainelli 
--
Florian



Re: [PATCH 09/22] mips/cpu: Expose play_dead()'s prototype definition

2023-02-06 Thread Florian Fainelli

On 2/3/23 14:05, Josh Poimboeuf wrote:

Include  to make sure play_dead() matches its prototype going
forward.

Signed-off-by: Josh Poimboeuf 


Acked-by: Florian Fainelli 
--
Florian



Re: [PATCH net-next 03/10] net: mdio: mux-bcm-iproc: Separate C22 and C45 transactions

2023-01-12 Thread Florian Fainelli

On 1/12/23 07:15, Michael Walle wrote:

From: Andrew Lunn 

The MDIO mux broadcom iproc can perform both C22 and C45 transfers.
Create separate functions for each and register the C45 versions using
the new API calls.

Signed-off-by: Andrew Lunn 
Signed-off-by: Michael Walle 
---
Apparently, in the c45 case, the reg value including the MII_ADDR_C45
bit is written to the hardware. Looks weird, that a "random" software
bit is written to a register. Florian is that correct? Also, with this
patch this flag isn't set anymore.


We should be masking the MII_ADDR_C45 bit because the MDIO_ADDR_OFFSET 
only defines bits 0 through 20 as being read/write and bits above being 
read-only. In practice, this is probably not making any difference or harm.

--
Florian



Re: [PATCH V12 01/20] uapi: simplify __ARCH_FLOCK{,64}_PAD a little

2022-07-14 Thread Florian Fainelli




On 4/5/2022 12:12 AM, guo...@kernel.org wrote:

From: Christoph Hellwig 

Don't bother to define the symbols empty, just don't use them.
That makes the intent a little more clear.

Remove the unused HAVE_ARCH_STRUCT_FLOCK64 define and merge the
32-bit mips struct flock into the generic one.

Add a new __ARCH_FLOCK_EXTRA_SYSID macro following the style of
__ARCH_FLOCK_PAD to avoid having a separate definition just for
one architecture.

Signed-off-by: Christoph Hellwig 
Signed-off-by: Guo Ren 
Reviewed-by: Arnd Bergmann 
Tested-by: Heiko Stuebner 


Being late to this, but this breaks the perf build for me using a MIPS 
toolchain with the following:


  CC 
/home/fainelli/work/buildroot/output/bmips/build/linux-custom/tools/perf/trace/beauty/fcntl.o
In file included from 
../../../../host/mipsel-buildroot-linux-gnu/sysroot/usr/include/asm/fcntl.h:77,

 from ../include/uapi/linux/fcntl.h:5,
 from trace/beauty/fcntl.c:10:
../include/uapi/asm-generic/fcntl.h:188:8: error: redefinition of 
'struct flock'

 struct flock {
^
In file included from ../include/uapi/linux/fcntl.h:5,
 from trace/beauty/fcntl.c:10:
../../../../host/mipsel-buildroot-linux-gnu/sysroot/usr/include/asm/fcntl.h:63:8: 
note: originally defined here

 struct flock {
^
make[6]: *** 
[/home/fainelli/work/buildroot/output/bmips/build/linux-custom/tools/build/Makefile.build:97: 
/home/fainelli/work/buildroot/output/bmips/build/linux-custom/tools/perf/trace/beauty/fcntl.o] 
Error 1


the kernel headers are set to 4.1.31 which is arguably old but 
toolchains using newer kernel headers do not fare much better either 
unfortunately as I tried a toolchain with kernel headers 4.9.x.


I will start doing more regular MIPS builds of the perf tools since that 
seems to escape our testing.


Thanks!
--
Florian


Re: [PATCH 06/30] soc: bcm: brcmstb: Document panic notifier action and remove useless header

2022-05-02 Thread Florian Fainelli




On 4/27/2022 3:49 PM, Guilherme G. Piccoli wrote:

The panic notifier of this driver is very simple code-wise, just a memory
write to a special position with some numeric code. But this is not clear
from the semantic point-of-view, and there is no public documentation
about that either.

After discussing this in the mailing-lists [0] and having Florian explained
it very well, this patch just document that in the code for the future
generations asking the same questions. Also, it removes a useless header.

[0] https://lore.kernel.org/lkml/781cafb0-8d06-8b56-907a-5175c2da1...@gmail.com

Fixes: 0b741b8234c8 ("soc: bcm: brcmstb: Add support for S2/S3/S5 suspend states 
(ARM)")
Cc: Brian Norris 
Cc: Doug Berger 
Cc: Florian Fainelli 
Cc: Justin Chen 
Cc: Lee Jones 
Cc: Markus Mayer 
Signed-off-by: Guilherme G. Piccoli 


Acked-by: Florian Fainelli 

Likewise, I am not sure if the Fixes tag is necessary here.
--
Florian


Re: [PATCH 15/30] bus: brcmstb_gisb: Clean-up panic/die notifiers

2022-05-02 Thread Florian Fainelli




On 4/27/2022 3:49 PM, Guilherme G. Piccoli wrote:

This patch improves the panic/die notifiers in this driver by
making use of a passed "id" instead of comparing pointer
address; also, it removes an useless prototype declaration
and unnecessary header inclusion.

This is part of a panic notifiers refactor - this notifier in
the future will be moved to a new list, that encompass the
information notifiers only.

Fixes: 9eb60880d9a9 ("bus: brcmstb_gisb: add notifier handling")
Cc: Brian Norris 
Cc: Florian Fainelli 
Signed-off-by: Guilherme G. Piccoli 


Acked-by: Florian Fainelli 

Not sure if the Fixes tag is warranted however as this is a clean up, 
and not really fixing a bug.

--
Florian


Re: [PATCH net-next v4 02/18] net: dsa: sja1105: remove use of iterator after list_for_each_entry() loop

2022-04-16 Thread Florian Fainelli




On 4/15/2022 5:29 AM, Jakob Koschel wrote:

From: Vladimir Oltean 

The link below explains that there is a desire to syntactically change
list_for_each_entry() and list_for_each() such that it becomes
impossible to use the iterator variable outside the scope of the loop.

Although sja1105_insert_gate_entry() makes legitimate use of the
iterator pointer when it breaks out, the pattern it uses may become
illegal, so it needs to change.

It is deemed acceptable to use a copy of the loop iterator, and
sja1105_insert_gate_entry() only needs to know the list_head element
before which the list insertion should be made. So let's profit from the
occasion and refactor the list iteration to a dedicated function.

An additional benefit is given by the fact that with the helper function
in place, we no longer need to special-case the empty list, since it is
equivalent to not having found any gating entry larger than the
specified interval in the list. We just need to insert at the tail of
that list (list_add vs list_add_tail on an empty list does the same
thing).

Link: 
https://patchwork.kernel.org/project/netdevbpf/patch/20220407102900.3086255-3-jakobkosc...@gmail.com/#24810127
Signed-off-by: Vladimir Oltean 
Signed-off-by: Jakob Koschel 


Reviewed-by: Florian Fainelli 
--
Florian


Re: [PATCH net-next v4 03/18] net: dsa: sja1105: reorder sja1105_first_entry_longer_than with memory allocation

2022-04-16 Thread Florian Fainelli




On 4/15/2022 5:29 AM, Jakob Koschel wrote:

From: Vladimir Oltean 

sja1105_first_entry_longer_than() does not make use of the full struct
sja1105_gate_entry *e, just of e->interval which is set from the passed
entry_time.

This means that if there is a gate conflict, we have allocated e for
nothing, just to free it later. Reorder the memory allocation and the
function call, to avoid that and simplify the error unwind path.

Signed-off-by: Vladimir Oltean 
Signed-off-by: Jakob Koschel 


Reviewed-by: Florian Fainelli 
--
Florian


Re: [PATCH net-next v4 04/18] net: dsa: sja1105: use list_add_tail(pos) instead of list_add(pos->prev)

2022-04-16 Thread Florian Fainelli




On 4/15/2022 5:29 AM, Jakob Koschel wrote:

From: Vladimir Oltean 

When passed a non-head list element, list_add_tail() actually adds the
new element to its left, which is what we want. Despite the slightly
confusing name, use the dedicated function which does the same thing as
the open-coded list_add(pos->prev).

Suggested-by: Jakub Kicinski 
Signed-off-by: Vladimir Oltean 
Signed-off-by: Jakob Koschel 


Reviewed-by: Florian Fainelli 
--
Florian


Re: [PATCH net-next v4 06/18] net: dsa: mv88e6xxx: refactor mv88e6xxx_port_vlan()

2022-04-16 Thread Florian Fainelli




On 4/15/2022 5:29 AM, Jakob Koschel wrote:

From: Vladimir Oltean 

To avoid bugs and speculative execution exploits due to type-confused
pointers at the end of a list_for_each_entry() loop, one measure is to
restrict code to not use the iterator variable outside the loop block.

In the case of mv88e6xxx_port_vlan(), this isn't a problem, as we never
let the loops exit through "natural causes" anyway, by using a "found"
variable and then using the last "dp" iterator prior to the break, which
is a safe thing to do.

Nonetheless, with the expected new syntax, this pattern will no longer
be possible.

Profit off of the occasion and break the two port finding methods into
smaller sub-functions. Somehow, returning a copy of the iterator pointer
is still accepted.

This change makes it redundant to have a "bool found", since the "dp"
from mv88e6xxx_port_vlan() now holds NULL if we haven't found what we
were looking for.

Signed-off-by: Vladimir Oltean 
Signed-off-by: Jakob Koschel 


Reviewed-by: Florian Fainelli 
--
Florian


Re: [PATCH net-next v4 05/18] net: dsa: mv88e6xxx: remove redundant check in mv88e6xxx_port_vlan()

2022-04-16 Thread Florian Fainelli




On 4/15/2022 5:29 AM, Jakob Koschel wrote:

From: Vladimir Oltean 

We know that "dev > dst->last_switch" in the "else" block.
In other words, that "dev - dst->last_switch" is > 0.

dsa_port_bridge_num_get(dp) can be 0, but the check
"if (bridge_num + dst->last_switch != dev) continue", rewritten as
"if (bridge_num != dev - dst->last_switch) continue", aka
"if (bridge_num != something which cannot be 0) continue",
makes it redundant to have the extra "if (!bridge_num) continue" logic,
since a bridge_num of zero would have been skipped anyway.

Signed-off-by: Vladimir Oltean 
Signed-off-by: Jakob Koschel 


Reviewed-by: Florian Fainelli 
--
Florian


Re: [PATCH net-next v4 07/18] net: dsa: Replace usage of found with dedicated list iterator variable

2022-04-16 Thread Florian Fainelli




On 4/15/2022 5:29 AM, Jakob Koschel wrote:

To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.

To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].

This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.

Link: 
https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=ehreask5sqxpwr9y7k9sa6cwx...@mail.gmail.com/
 [1]
Signed-off-by: Jakob Koschel 


Reviewed-by: Florian Fainelli 
--
Florian


[PATCH stable 4.9 2/2] arch: pgtable: define MAX_POSSIBLE_PHYSMEM_BITS where needed

2021-11-03 Thread Florian Fainelli
From: Arnd Bergmann 

[ Upstream commit cef397038167ac15d085914493d6c86385773709 ]

Stefan Agner reported a bug when using zsram on 32-bit Arm machines
with RAM above the 4GB address boundary:

  Unable to handle kernel NULL pointer dereference at virtual address 
  pgd = a27bd01c
  [] *pgd=236a0003, *pmd=1ffa64003
  Internal error: Oops: 207 [#1] SMP ARM
  Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil 
raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
  CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
  Hardware name: BCM2711
  PC is at zs_map_object+0x94/0x338
  LR is at zram_bvec_rw.constprop.0+0x330/0xa64
  pc : []lr : []psr: 6013
  sp : e376bbe0  ip :   fp : c1e2921c
  r10: 0002  r9 : c1dda730  r8 : 
  r7 : e8ff7a00  r6 :   r5 : 02f9ffa0  r4 : e371
  r3 : 000fdffe  r2 : c1e0ce80  r1 : ebf979a0  r0 : 
  Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
  Control: 30c5383d  Table: 235c2a80  DAC: fffd
  Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
  Stack: (0xe376bbe0 to 0xe376c000)

As it turns out, zsram needs to know the maximum memory size, which
is defined in MAX_PHYSMEM_BITS when CONFIG_SPARSEMEM is set, or in
MAX_POSSIBLE_PHYSMEM_BITS on the x86 architecture.

The same problem will be hit on all 32-bit architectures that have a
physical address space larger than 4GB and happen to not enable sparsemem
and include asm/sparsemem.h from asm/pgtable.h.

After the initial discussion, I suggested just always defining
MAX_POSSIBLE_PHYSMEM_BITS whenever CONFIG_PHYS_ADDR_T_64BIT is
set, or provoking a build error otherwise. This addresses all
configurations that can currently have this runtime bug, but
leaves all other configurations unchanged.

I looked up the possible number of bits in source code and
datasheets, here is what I found:

 - on ARC, CONFIG_ARC_HAS_PAE40 controls whether 32 or 40 bits are used
 - on ARM, CONFIG_LPAE enables 40 bit addressing, without it we never
   support more than 32 bits, even though supersections in theory allow
   up to 40 bits as well.
 - on MIPS, some MIPS32r1 or later chips support 36 bits, and MIPS32r5
   XPA supports up to 60 bits in theory, but 40 bits are more than
   anyone will ever ship
 - On PowerPC, there are three different implementations of 36 bit
   addressing, but 32-bit is used without CONFIG_PTE_64BIT
 - On RISC-V, the normal page table format can support 34 bit
   addressing. There is no highmem support on RISC-V, so anything
   above 2GB is unused, but it might be useful to eventually support
   CONFIG_ZRAM for high pages.

Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
Fixes: 02390b87a945 ("mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS")
Acked-by: Thomas Bogendoerfer 
Reviewed-by: Stefan Agner 
Tested-by: Stefan Agner 
Acked-by: Mike Rapoport 
Link: 
https://lore.kernel.org/linux-mm/bdfa44bf1c570b05d6c70898e2bbb0acf234ecdf.1604762181.git.ste...@agner.ch/
Signed-off-by: Arnd Bergmann 
Signed-off-by: Sasha Levin 
[florian: patch arch/powerpc/include/asm/pte-common.h for 4.9.y
removed arch/riscv/include/asm/pgtable.h which does not exist]
Signed-off-by: Florian Fainelli 
---
 arch/arc/include/asm/pgtable.h|  2 ++
 arch/arm/include/asm/pgtable-2level.h |  2 ++
 arch/arm/include/asm/pgtable-3level.h |  2 ++
 arch/mips/include/asm/pgtable-32.h|  3 +++
 arch/powerpc/include/asm/pte-common.h |  2 ++
 include/asm-generic/pgtable.h | 13 +
 6 files changed, 24 insertions(+)

diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h
index c10f5cb203e6..81198a6773c6 100644
--- a/arch/arc/include/asm/pgtable.h
+++ b/arch/arc/include/asm/pgtable.h
@@ -137,8 +137,10 @@
 
 #ifdef CONFIG_ARC_HAS_PAE40
 #define PTE_BITS_NON_RWX_IN_PD1(0xff | PAGE_MASK | 
_PAGE_CACHEABLE)
+#define MAX_POSSIBLE_PHYSMEM_BITS 40
 #else
 #define PTE_BITS_NON_RWX_IN_PD1(PAGE_MASK | _PAGE_CACHEABLE)
+#define MAX_POSSIBLE_PHYSMEM_BITS 32
 #endif
 
 /**
diff --git a/arch/arm/include/asm/pgtable-2level.h 
b/arch/arm/include/asm/pgtable-2level.h
index 92fd2c8a9af0..6154902bed83 100644
--- a/arch/arm/include/asm/pgtable-2level.h
+++ b/arch/arm/include/asm/pgtable-2level.h
@@ -78,6 +78,8 @@
 #define PTE_HWTABLE_OFF(PTE_HWTABLE_PTRS * sizeof(pte_t))
 #define PTE_HWTABLE_SIZE   (PTRS_PER_PTE * sizeof(u32))
 
+#define MAX_POSSIBLE_PHYSMEM_BITS  32
+
 /*
  * PMD_SHIFT determines the size of the area a second-level page table can map
  * PGDIR_SHIFT determines what a third-level page table entry can map
diff --git a/arch/arm/include/asm/pgtable-3level.h 
b/arch/arm/include/asm/pgtable-3level.h
index 2a029bceaf2f..35807e611b6e 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -37

[PATCH stable 4.9 1/2] mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS

2021-11-03 Thread Florian Fainelli
From: "Kirill A. Shutemov" 

commit 02390b87a9459937cdb299e6b34ff33992512ec7 upstream

With boot-time switching between paging mode we will have variable
MAX_PHYSMEM_BITS.

Let's use the maximum variable possible for CONFIG_X86_5LEVEL=y
configuration to define zsmalloc data structures.

The patch introduces MAX_POSSIBLE_PHYSMEM_BITS to cover such case.
It also suits well to handle PAE special case.

Signed-off-by: Kirill A. Shutemov 
Reviewed-by: Nitin Gupta 
Acked-by: Minchan Kim 
Cc: Andy Lutomirski 
Cc: Borislav Petkov 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Sergey Senozhatsky 
Cc: Thomas Gleixner 
Cc: linux...@kvack.org
Link: 
http://lkml.kernel.org/r/20180214111656.88514-3-kirill.shute...@linux.intel.com
Signed-off-by: Ingo Molnar 
[florian: drop arch/x86/include/asm/pgtable_64_types.h changes since
there is no CONFIG_X86_5LEVEL]
Signed-off-by: Florian Fainelli 
---
 arch/x86/include/asm/pgtable-3level_types.h |  1 +
 mm/zsmalloc.c   | 13 +++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/pgtable-3level_types.h 
b/arch/x86/include/asm/pgtable-3level_types.h
index bcc89625ebe5..f3f719d59e61 100644
--- a/arch/x86/include/asm/pgtable-3level_types.h
+++ b/arch/x86/include/asm/pgtable-3level_types.h
@@ -42,5 +42,6 @@ typedef union {
  */
 #define PTRS_PER_PTE   512
 
+#define MAX_POSSIBLE_PHYSMEM_BITS  36
 
 #endif /* _ASM_X86_PGTABLE_3LEVEL_DEFS_H */
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 8db3c2b27a17..2b7bfd97587a 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -83,18 +83,19 @@
  * This is made more complicated by various memory models and PAE.
  */
 
-#ifndef MAX_PHYSMEM_BITS
-#ifdef CONFIG_HIGHMEM64G
-#define MAX_PHYSMEM_BITS 36
-#else /* !CONFIG_HIGHMEM64G */
+#ifndef MAX_POSSIBLE_PHYSMEM_BITS
+#ifdef MAX_PHYSMEM_BITS
+#define MAX_POSSIBLE_PHYSMEM_BITS MAX_PHYSMEM_BITS
+#else
 /*
  * If this definition of MAX_PHYSMEM_BITS is used, OBJ_INDEX_BITS will just
  * be PAGE_SHIFT
  */
-#define MAX_PHYSMEM_BITS BITS_PER_LONG
+#define MAX_POSSIBLE_PHYSMEM_BITS BITS_PER_LONG
 #endif
 #endif
-#define _PFN_BITS  (MAX_PHYSMEM_BITS - PAGE_SHIFT)
+
+#define _PFN_BITS  (MAX_POSSIBLE_PHYSMEM_BITS - PAGE_SHIFT)
 
 /*
  * Memory for allocating for handle keeps object position by
-- 
2.25.1



[PATCH stable 4.9 0/2] zsmalloc MAX_POSSIBLE_PHYSMEM_BITS

2021-11-03 Thread Florian Fainelli
This patch series is a back port necessary to address the problem
reported by Stefan Agner:

https://lore.kernel.org/linux-mm/bdfa44bf1c570b05d6c70898e2bbb0acf234ecdf.1604762181.git.ste...@agner.ch/

but which ended up being addressed by Arnd in a slightly different way
from Stefan's submission.

The first patch from Kirill is back ported in order to have
MAX_POSSIBLE_PHYSMEM_BITS be acted on my the zsmalloc.c code.

Arnd Bergmann (1):
  arch: pgtable: define MAX_POSSIBLE_PHYSMEM_BITS where needed

Kirill A. Shutemov (1):
  mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS

 arch/arc/include/asm/pgtable.h  |  2 ++
 arch/arm/include/asm/pgtable-2level.h   |  2 ++
 arch/arm/include/asm/pgtable-3level.h   |  2 ++
 arch/mips/include/asm/pgtable-32.h  |  3 +++
 arch/powerpc/include/asm/pte-common.h   |  2 ++
 arch/x86/include/asm/pgtable-3level_types.h |  1 +
 include/asm-generic/pgtable.h   | 13 +
 mm/zsmalloc.c   | 13 +++--
 8 files changed, 32 insertions(+), 6 deletions(-)

-- 
2.25.1



[PATCH stable 4.14 2/2] arch: pgtable: define MAX_POSSIBLE_PHYSMEM_BITS where needed

2021-11-03 Thread Florian Fainelli
From: Arnd Bergmann 

[ Upstream commit cef397038167ac15d085914493d6c86385773709 ]

Stefan Agner reported a bug when using zsram on 32-bit Arm machines
with RAM above the 4GB address boundary:

  Unable to handle kernel NULL pointer dereference at virtual address 
  pgd = a27bd01c
  [] *pgd=236a0003, *pmd=1ffa64003
  Internal error: Oops: 207 [#1] SMP ARM
  Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil 
raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
  CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
  Hardware name: BCM2711
  PC is at zs_map_object+0x94/0x338
  LR is at zram_bvec_rw.constprop.0+0x330/0xa64
  pc : []lr : []psr: 6013
  sp : e376bbe0  ip :   fp : c1e2921c
  r10: 0002  r9 : c1dda730  r8 : 
  r7 : e8ff7a00  r6 :   r5 : 02f9ffa0  r4 : e371
  r3 : 000fdffe  r2 : c1e0ce80  r1 : ebf979a0  r0 : 
  Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
  Control: 30c5383d  Table: 235c2a80  DAC: fffd
  Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
  Stack: (0xe376bbe0 to 0xe376c000)

As it turns out, zsram needs to know the maximum memory size, which
is defined in MAX_PHYSMEM_BITS when CONFIG_SPARSEMEM is set, or in
MAX_POSSIBLE_PHYSMEM_BITS on the x86 architecture.

The same problem will be hit on all 32-bit architectures that have a
physical address space larger than 4GB and happen to not enable sparsemem
and include asm/sparsemem.h from asm/pgtable.h.

After the initial discussion, I suggested just always defining
MAX_POSSIBLE_PHYSMEM_BITS whenever CONFIG_PHYS_ADDR_T_64BIT is
set, or provoking a build error otherwise. This addresses all
configurations that can currently have this runtime bug, but
leaves all other configurations unchanged.

I looked up the possible number of bits in source code and
datasheets, here is what I found:

 - on ARC, CONFIG_ARC_HAS_PAE40 controls whether 32 or 40 bits are used
 - on ARM, CONFIG_LPAE enables 40 bit addressing, without it we never
   support more than 32 bits, even though supersections in theory allow
   up to 40 bits as well.
 - on MIPS, some MIPS32r1 or later chips support 36 bits, and MIPS32r5
   XPA supports up to 60 bits in theory, but 40 bits are more than
   anyone will ever ship
 - On PowerPC, there are three different implementations of 36 bit
   addressing, but 32-bit is used without CONFIG_PTE_64BIT
 - On RISC-V, the normal page table format can support 34 bit
   addressing. There is no highmem support on RISC-V, so anything
   above 2GB is unused, but it might be useful to eventually support
   CONFIG_ZRAM for high pages.

Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
Fixes: 02390b87a945 ("mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS")
Acked-by: Thomas Bogendoerfer 
Reviewed-by: Stefan Agner 
Tested-by: Stefan Agner 
Acked-by: Mike Rapoport 
Link: 
https://lore.kernel.org/linux-mm/bdfa44bf1c570b05d6c70898e2bbb0acf234ecdf.1604762181.git.ste...@agner.ch/
Signed-off-by: Arnd Bergmann 
Signed-off-by: Sasha Levin 
[florian: patch arch/powerpc/include/asm/pte-common.h for 4.14.y
removed arch/riscv/include/asm/pgtable.h which does not exist]
Signed-off-by: Florian Fainelli 
---
 arch/arc/include/asm/pgtable.h|  2 ++
 arch/arm/include/asm/pgtable-2level.h |  2 ++
 arch/arm/include/asm/pgtable-3level.h |  2 ++
 arch/mips/include/asm/pgtable-32.h|  3 +++
 arch/powerpc/include/asm/pte-common.h |  2 ++
 include/asm-generic/pgtable.h | 13 +
 6 files changed, 24 insertions(+)

diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h
index 77676e18da69..a31ae69da639 100644
--- a/arch/arc/include/asm/pgtable.h
+++ b/arch/arc/include/asm/pgtable.h
@@ -138,8 +138,10 @@
 
 #ifdef CONFIG_ARC_HAS_PAE40
 #define PTE_BITS_NON_RWX_IN_PD1(0xff | PAGE_MASK | 
_PAGE_CACHEABLE)
+#define MAX_POSSIBLE_PHYSMEM_BITS 40
 #else
 #define PTE_BITS_NON_RWX_IN_PD1(PAGE_MASK | _PAGE_CACHEABLE)
+#define MAX_POSSIBLE_PHYSMEM_BITS 32
 #endif
 
 /**
diff --git a/arch/arm/include/asm/pgtable-2level.h 
b/arch/arm/include/asm/pgtable-2level.h
index 92fd2c8a9af0..6154902bed83 100644
--- a/arch/arm/include/asm/pgtable-2level.h
+++ b/arch/arm/include/asm/pgtable-2level.h
@@ -78,6 +78,8 @@
 #define PTE_HWTABLE_OFF(PTE_HWTABLE_PTRS * sizeof(pte_t))
 #define PTE_HWTABLE_SIZE   (PTRS_PER_PTE * sizeof(u32))
 
+#define MAX_POSSIBLE_PHYSMEM_BITS  32
+
 /*
  * PMD_SHIFT determines the size of the area a second-level page table can map
  * PGDIR_SHIFT determines what a third-level page table entry can map
diff --git a/arch/arm/include/asm/pgtable-3level.h 
b/arch/arm/include/asm/pgtable-3level.h
index 2a029bceaf2f..35807e611b6e 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -37

[PATCH stable 4.14 1/2] mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS

2021-11-03 Thread Florian Fainelli
From: "Kirill A. Shutemov" 

commit 02390b87a9459937cdb299e6b34ff33992512ec7 upstream

With boot-time switching between paging mode we will have variable
MAX_PHYSMEM_BITS.

Let's use the maximum variable possible for CONFIG_X86_5LEVEL=y
configuration to define zsmalloc data structures.

The patch introduces MAX_POSSIBLE_PHYSMEM_BITS to cover such case.
It also suits well to handle PAE special case.

Signed-off-by: Kirill A. Shutemov 
Reviewed-by: Nitin Gupta 
Acked-by: Minchan Kim 
Cc: Andy Lutomirski 
Cc: Borislav Petkov 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Sergey Senozhatsky 
Cc: Thomas Gleixner 
Cc: linux...@kvack.org
Link: 
http://lkml.kernel.org/r/20180214111656.88514-3-kirill.shute...@linux.intel.com
Signed-off-by: Ingo Molnar 
Signed-off-by: Florian Fainelli 
---
 arch/x86/include/asm/pgtable-3level_types.h |  1 +
 arch/x86/include/asm/pgtable_64_types.h |  2 ++
 mm/zsmalloc.c   | 13 +++--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/pgtable-3level_types.h 
b/arch/x86/include/asm/pgtable-3level_types.h
index 876b4c77d983..6a59a6d0cc50 100644
--- a/arch/x86/include/asm/pgtable-3level_types.h
+++ b/arch/x86/include/asm/pgtable-3level_types.h
@@ -44,5 +44,6 @@ typedef union {
  */
 #define PTRS_PER_PTE   512
 
+#define MAX_POSSIBLE_PHYSMEM_BITS  36
 
 #endif /* _ASM_X86_PGTABLE_3LEVEL_DEFS_H */
diff --git a/arch/x86/include/asm/pgtable_64_types.h 
b/arch/x86/include/asm/pgtable_64_types.h
index bf6d2692fc60..2bd79b7ae9d6 100644
--- a/arch/x86/include/asm/pgtable_64_types.h
+++ b/arch/x86/include/asm/pgtable_64_types.h
@@ -40,6 +40,8 @@ typedef struct { pteval_t pte; } pte_t;
 #define P4D_SIZE   (_AC(1, UL) << P4D_SHIFT)
 #define P4D_MASK   (~(P4D_SIZE - 1))
 
+#define MAX_POSSIBLE_PHYSMEM_BITS  52
+
 #else /* CONFIG_X86_5LEVEL */
 
 /*
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 6ed736ea9b59..633ebcac82f8 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -83,18 +83,19 @@
  * This is made more complicated by various memory models and PAE.
  */
 
-#ifndef MAX_PHYSMEM_BITS
-#ifdef CONFIG_HIGHMEM64G
-#define MAX_PHYSMEM_BITS 36
-#else /* !CONFIG_HIGHMEM64G */
+#ifndef MAX_POSSIBLE_PHYSMEM_BITS
+#ifdef MAX_PHYSMEM_BITS
+#define MAX_POSSIBLE_PHYSMEM_BITS MAX_PHYSMEM_BITS
+#else
 /*
  * If this definition of MAX_PHYSMEM_BITS is used, OBJ_INDEX_BITS will just
  * be PAGE_SHIFT
  */
-#define MAX_PHYSMEM_BITS BITS_PER_LONG
+#define MAX_POSSIBLE_PHYSMEM_BITS BITS_PER_LONG
 #endif
 #endif
-#define _PFN_BITS  (MAX_PHYSMEM_BITS - PAGE_SHIFT)
+
+#define _PFN_BITS  (MAX_POSSIBLE_PHYSMEM_BITS - PAGE_SHIFT)
 
 /*
  * Memory for allocating for handle keeps object position by
-- 
2.25.1



[PATCH stable 4.14 0/2] zsmalloc MAX_POSSIBLE_PHYSMEM_BITS

2021-11-03 Thread Florian Fainelli
This patch series is a back port necessary to address the problem
reported by Stefan Agner:

https://lore.kernel.org/linux-mm/bdfa44bf1c570b05d6c70898e2bbb0acf234ecdf.1604762181.git.ste...@agner.ch/

but which ended up being addressed by Arnd in a slightly different way
from Stefan's submission.

The first patch from Kirill is back ported in order to have
MAX_POSSIBLE_PHYSMEM_BITS be acted on my the zsmalloc.c code.

Arnd Bergmann (1):
  arch: pgtable: define MAX_POSSIBLE_PHYSMEM_BITS where needed

Kirill A. Shutemov (1):
  mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS

 arch/arc/include/asm/pgtable.h  |  2 ++
 arch/arm/include/asm/pgtable-2level.h   |  2 ++
 arch/arm/include/asm/pgtable-3level.h   |  2 ++
 arch/mips/include/asm/pgtable-32.h  |  3 +++
 arch/powerpc/include/asm/pte-common.h   |  2 ++
 arch/x86/include/asm/pgtable-3level_types.h |  1 +
 arch/x86/include/asm/pgtable_64_types.h |  2 ++
 include/asm-generic/pgtable.h   | 13 +
 mm/zsmalloc.c   | 13 +++--
 9 files changed, 34 insertions(+), 6 deletions(-)

-- 
2.25.1



[PATCH stable 4.19 1/1] arch: pgtable: define MAX_POSSIBLE_PHYSMEM_BITS where needed

2021-11-03 Thread Florian Fainelli
From: Arnd Bergmann 

[ Upstream commit cef397038167ac15d085914493d6c86385773709 ]

Stefan Agner reported a bug when using zsram on 32-bit Arm machines
with RAM above the 4GB address boundary:

  Unable to handle kernel NULL pointer dereference at virtual address 
  pgd = a27bd01c
  [] *pgd=236a0003, *pmd=1ffa64003
  Internal error: Oops: 207 [#1] SMP ARM
  Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil 
raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
  CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
  Hardware name: BCM2711
  PC is at zs_map_object+0x94/0x338
  LR is at zram_bvec_rw.constprop.0+0x330/0xa64
  pc : []lr : []psr: 6013
  sp : e376bbe0  ip :   fp : c1e2921c
  r10: 0002  r9 : c1dda730  r8 : 
  r7 : e8ff7a00  r6 :   r5 : 02f9ffa0  r4 : e371
  r3 : 000fdffe  r2 : c1e0ce80  r1 : ebf979a0  r0 : 
  Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
  Control: 30c5383d  Table: 235c2a80  DAC: fffd
  Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
  Stack: (0xe376bbe0 to 0xe376c000)

As it turns out, zsram needs to know the maximum memory size, which
is defined in MAX_PHYSMEM_BITS when CONFIG_SPARSEMEM is set, or in
MAX_POSSIBLE_PHYSMEM_BITS on the x86 architecture.

The same problem will be hit on all 32-bit architectures that have a
physical address space larger than 4GB and happen to not enable sparsemem
and include asm/sparsemem.h from asm/pgtable.h.

After the initial discussion, I suggested just always defining
MAX_POSSIBLE_PHYSMEM_BITS whenever CONFIG_PHYS_ADDR_T_64BIT is
set, or provoking a build error otherwise. This addresses all
configurations that can currently have this runtime bug, but
leaves all other configurations unchanged.

I looked up the possible number of bits in source code and
datasheets, here is what I found:

 - on ARC, CONFIG_ARC_HAS_PAE40 controls whether 32 or 40 bits are used
 - on ARM, CONFIG_LPAE enables 40 bit addressing, without it we never
   support more than 32 bits, even though supersections in theory allow
   up to 40 bits as well.
 - on MIPS, some MIPS32r1 or later chips support 36 bits, and MIPS32r5
   XPA supports up to 60 bits in theory, but 40 bits are more than
   anyone will ever ship
 - On PowerPC, there are three different implementations of 36 bit
   addressing, but 32-bit is used without CONFIG_PTE_64BIT
 - On RISC-V, the normal page table format can support 34 bit
   addressing. There is no highmem support on RISC-V, so anything
   above 2GB is unused, but it might be useful to eventually support
   CONFIG_ZRAM for high pages.

Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
Fixes: 02390b87a945 ("mm/zsmalloc: Prepare to variable MAX_PHYSMEM_BITS")
Acked-by: Thomas Bogendoerfer 
Reviewed-by: Stefan Agner 
Tested-by: Stefan Agner 
Acked-by: Mike Rapoport 
Link: 
https://lore.kernel.org/linux-mm/bdfa44bf1c570b05d6c70898e2bbb0acf234ecdf.1604762181.git.ste...@agner.ch/
Signed-off-by: Arnd Bergmann 
Signed-off-by: Sasha Levin 
[florian: patch arch/powerpc/include/asm/pte-common.h for 4.19.y]
Signed-off-by: Florian Fainelli 
---
 arch/arc/include/asm/pgtable.h|  2 ++
 arch/arm/include/asm/pgtable-2level.h |  2 ++
 arch/arm/include/asm/pgtable-3level.h |  2 ++
 arch/mips/include/asm/pgtable-32.h|  3 +++
 arch/powerpc/include/asm/pte-common.h |  2 ++
 arch/riscv/include/asm/pgtable-32.h   |  2 ++
 include/asm-generic/pgtable.h | 13 +
 7 files changed, 26 insertions(+)

diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h
index cf4be70d5892..f231963b4011 100644
--- a/arch/arc/include/asm/pgtable.h
+++ b/arch/arc/include/asm/pgtable.h
@@ -138,8 +138,10 @@
 
 #ifdef CONFIG_ARC_HAS_PAE40
 #define PTE_BITS_NON_RWX_IN_PD1(0xff | PAGE_MASK | 
_PAGE_CACHEABLE)
+#define MAX_POSSIBLE_PHYSMEM_BITS 40
 #else
 #define PTE_BITS_NON_RWX_IN_PD1(PAGE_MASK | _PAGE_CACHEABLE)
+#define MAX_POSSIBLE_PHYSMEM_BITS 32
 #endif
 
 /**
diff --git a/arch/arm/include/asm/pgtable-2level.h 
b/arch/arm/include/asm/pgtable-2level.h
index 12659ce5c1f3..90bf19d99378 100644
--- a/arch/arm/include/asm/pgtable-2level.h
+++ b/arch/arm/include/asm/pgtable-2level.h
@@ -78,6 +78,8 @@
 #define PTE_HWTABLE_OFF(PTE_HWTABLE_PTRS * sizeof(pte_t))
 #define PTE_HWTABLE_SIZE   (PTRS_PER_PTE * sizeof(u32))
 
+#define MAX_POSSIBLE_PHYSMEM_BITS  32
+
 /*
  * PMD_SHIFT determines the size of the area a second-level page table can map
  * PGDIR_SHIFT determines what a third-level page table entry can map
diff --git a/arch/arm/include/asm/pgtable-3level.h 
b/arch/arm/include/asm/pgtable-3level.h
index 6d50a11d7793..7ba08dd650e3 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -37,6 +37,8 @@
 #de

Re: [PATCH 03/12] ARM: broadcom: Use of_get_cpu_hwid()

2021-10-06 Thread Florian Fainelli




On 10/6/2021 9:43 AM, Rob Herring wrote:

Replace open coded parsing of CPU nodes 'reg' property with
of_get_cpu_hwid().

Cc: Florian Fainelli 
Cc: Ray Jui 
Cc: Scott Branden 
Cc: bcm-kernel-feedback-l...@broadcom.com
Cc: Russell King 
Signed-off-by: Rob Herring 


Acked-by: Florian Fainelli 
--
Florian


Re: [PATCH 00/12] DT: CPU h/w id parsing clean-ups and cacheinfo id support

2021-10-06 Thread Florian Fainelli




On 10/6/2021 9:43 AM, Rob Herring wrote:

The first 10 patches add a new function, of_get_cpu_hwid(), which parses
CPU DT node 'reg' property, and then use it to replace all the open
coded versions of parsing CPU node 'reg' properties.

The last 2 patches add support for populating the cacheinfo 'id' on DT
platforms. The minimum associated CPU hwid is used for the id. The id is
optional, but necessary for resctrl which is being adapted for Arm MPAM.

Tested on arm64. Compile tested on arm, x86 and powerpc.


On ARM and ARM64:

Tested-by: Florian Fainelli 

lscpu -C continues to work on ARM64 as before with cache properties 
provided in the FDT.

--
Florian


Re: [PATCH v7 02/15] swiotlb: Refactor swiotlb_create_debugfs

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Split the debugfs creation to make the code reusable for supporting
> different bounce buffer pools, e.g. restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 07/15] swiotlb: Update is_swiotlb_active to add a struct device argument

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Update is_swiotlb_active to add a struct device argument. This will be
> useful later to allow for restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 06/15] swiotlb: Update is_swiotlb_buffer to add a struct device argument

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Update is_swiotlb_buffer to add a struct device argument. This will be
> useful later to allow for restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 05/15] swiotlb: Add a new get_io_tlb_mem getter

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new getter, get_io_tlb_mem, to help select the io_tlb_mem struct.
> The restricted DMA pool is preferred if available.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 03/15] swiotlb: Add DMA_RESTRICTED_POOL

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new kconfig symbol, DMA_RESTRICTED_POOL, for restricted DMA pool.
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 11/15] dma-direct: Add a new wrapper __dma_direct_free_pages()

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new wrapper __dma_direct_free_pages() that will be useful later
> for swiotlb_free().
> 
> Signed-off-by: Claire Chang 

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 04/15] swiotlb: Add restricted DMA pool initialization

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add the initialization function to create restricted DMA pools from
> matching reserved-memory nodes.
> 
> Signed-off-by: Claire Chang 
> ---
>  include/linux/device.h  |  4 +++
>  include/linux/swiotlb.h |  3 +-
>  kernel/dma/swiotlb.c| 76 +
>  3 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 38a2071cf776..4987608ea4ff 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -416,6 +416,7 @@ struct dev_links_info {
>   * @dma_pools:   Dma pools (if dma'ble device).
>   * @dma_mem: Internal for coherent mem override.
>   * @cma_area:Contiguous memory area for dma allocations
> + * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override.
>   * @archdata:For arch-specific additions.
>   * @of_node: Associated device tree node.
>   * @fwnode:  Associated device node supplied by platform firmware.
> @@ -521,6 +522,9 @@ struct device {
>  #ifdef CONFIG_DMA_CMA
>   struct cma *cma_area;   /* contiguous memory area for dma
>  allocations */
> +#endif
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> + struct io_tlb_mem *dma_io_tlb_mem;
>  #endif
>   /* arch specific additions */
>   struct dev_archdata archdata;
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 216854a5e513..03ad6e3b4056 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -72,7 +72,8 @@ extern enum swiotlb_force swiotlb_force;
>   *   range check to see if the memory was in fact allocated by this
>   *   API.
>   * @nslabs:  The number of IO TLB blocks (in groups of 64) between @start and
> - *   @end. This is command line adjustable via setup_io_tlb_npages.
> + *   @end. For default swiotlb, this is command line adjustable via
> + *   setup_io_tlb_npages.
>   * @used:The number of used IO TLB block.
>   * @list:The free list describing the number of free entries available
>   *   from each index.
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index b849b01a446f..1d8eb4de0d01 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -39,6 +39,13 @@
>  #ifdef CONFIG_DEBUG_FS
>  #include 
>  #endif
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
>  
>  #include 
>  #include 
> @@ -690,3 +697,72 @@ static int __init swiotlb_create_default_debugfs(void)
>  late_initcall(swiotlb_create_default_debugfs);
>  
>  #endif
> +
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> + struct device *dev)
> +{
> + struct io_tlb_mem *mem = rmem->priv;
> + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
> +
> + if (dev->dma_io_tlb_mem)
> + return 0;
> +
> + /*
> +  * Since multiple devices can share the same pool, the private data,
> +  * io_tlb_mem struct, will be initialized by the first device attached
> +  * to it.
> +  */
> + if (!mem) {
> + mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL);
> + if (!mem)
> + return -ENOMEM;
> +
> + if (PageHighMem(pfn_to_page(PHYS_PFN(rmem->base)))) {
> + kfree(mem);
> + return -EINVAL;

This could probably deserve a warning here to indicate that the reserved
area must be accessible within the linear mapping as I would expect a
lot of people to trip over that.

Reviewed-by: Florian Fainelli 
-- 
Florian


Re: [PATCH v7 01/15] swiotlb: Refactor swiotlb init functions

2021-05-19 Thread Florian Fainelli



On 5/17/2021 11:42 PM, Claire Chang wrote:
> Add a new function, swiotlb_init_io_tlb_mem, for the io_tlb_mem struct
> initialization to make the code reusable.
> 
> Note that we now also call set_memory_decrypted in swiotlb_init_with_tbl.
> 
> Signed-off-by: Claire Chang 
> ---
>  kernel/dma/swiotlb.c | 51 ++--
>  1 file changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 8ca7d505d61c..d3232fc19385 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -168,9 +168,30 @@ void __init swiotlb_update_mem_attributes(void)
>   memset(vaddr, 0, bytes);
>  }
>  
> -int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int 
> verbose)
> +static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t 
> start,
> + unsigned long nslabs, bool late_alloc)
>  {
> + void *vaddr = phys_to_virt(start);
>   unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
> +
> + mem->nslabs = nslabs;
> + mem->start = start;
> + mem->end = mem->start + bytes;
> + mem->index = 0;
> + mem->late_alloc = late_alloc;
> + spin_lock_init(>lock);
> + for (i = 0; i < mem->nslabs; i++) {
> + mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
> + mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
> + mem->slots[i].alloc_size = 0;
> + }
> +
> + set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
> + memset(vaddr, 0, bytes);

You are doing an unconditional set_memory_decrypted() followed by a
memset here, and then:

> +}
> +
> +int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int 
> verbose)
> +{
>   struct io_tlb_mem *mem;
>   size_t alloc_size;
>  
> @@ -186,16 +207,8 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned 
> long nslabs, int verbose)
>   if (!mem)
>   panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> __func__, alloc_size, PAGE_SIZE);
> - mem->nslabs = nslabs;
> - mem->start = __pa(tlb);
> - mem->end = mem->start + bytes;
> - mem->index = 0;
> - spin_lock_init(>lock);
> - for (i = 0; i < mem->nslabs; i++) {
> - mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
> - mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
> - mem->slots[i].alloc_size = 0;
> - }
> +
> + swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false);

You convert this call site with swiotlb_init_io_tlb_mem() which did not
do the set_memory_decrypted()+memset(). Is this okay or should
swiotlb_init_io_tlb_mem() add an additional argument to do this
conditionally?
-- 
Florian


Re: [PATCH net-next v3 2/2] of: net: fix of_get_mac_addr_nvmem() for PCI and DSA nodes

2021-04-06 Thread Florian Fainelli



On 4/6/2021 3:09 PM, Michael Walle wrote:
> of_get_mac_address() already supports fetching the MAC address by an
> nvmem provider. But until now, it was just working for platform devices.
> Esp. it was not working for DSA ports and PCI devices. It gets more
> common that PCI devices have a device tree binding since SoCs contain
> integrated root complexes.
> 
> Use the nvmem of_* binding to fetch the nvmem cells by a struct
> device_node. We still have to try to read the cell by device first
> because there might be a nvmem_cell_lookup associated with that device.
> 
> Signed-off-by: Michael Walle 
> ---
> Please note, that I've kept the nvmem_get_mac_address() which operates
> on a device. The new of_get_mac_addr_nvmem() is almost identical and
> there are no users of the former function right now, but it seems to be
> the "newer" version to get the MAC address for a "struct device". Thus
> I've kept it. Please advise, if I should kill it though.

Nit: if you need to resubmit you could rephrase the subject such that
the limitation of of_get_mac_addr_nvmem() is lifted to include all kinds
of devices, and no longer just platform_device instances as before.
-- 
Florian


Re: [PATCH v4 13/14] dt-bindings: of: Add restricted DMA pool

2021-03-10 Thread Florian Fainelli



On 3/10/2021 1:40 PM, Rob Herring wrote:
> On Wed, Mar 10, 2021 at 9:08 AM Will Deacon  wrote:
>>
>> Hi Claire,
>>
>> On Tue, Feb 09, 2021 at 02:21:30PM +0800, Claire Chang wrote:
>>> Introduce the new compatible string, restricted-dma-pool, for restricted
>>> DMA. One can specify the address and length of the restricted DMA memory
>>> region by restricted-dma-pool in the reserved-memory node.
>>>
>>> Signed-off-by: Claire Chang 
>>> ---
>>>  .../reserved-memory/reserved-memory.txt   | 24 +++
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
>>> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>>> index e8d3096d922c..fc9a12c2f679 100644
>>> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>>> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>>> @@ -51,6 +51,20 @@ compatible (optional) - standard definition
>>>used as a shared pool of DMA buffers for a set of devices. It can
>>>be used by an operating system to instantiate the necessary pool
>>>management subsystem if necessary.
>>> +- restricted-dma-pool: This indicates a region of memory meant to 
>>> be
>>> +  used as a pool of restricted DMA buffers for a set of devices. 
>>> The
>>> +  memory region would be the only region accessible to those 
>>> devices.
>>> +  When using this, the no-map and reusable properties must not be 
>>> set,
>>> +  so the operating system can create a virtual mapping that will 
>>> be used
>>> +  for synchronization. The main purpose for restricted DMA is to
>>> +  mitigate the lack of DMA access control on systems without an 
>>> IOMMU,
>>> +  which could result in the DMA accessing the system memory at
>>> +  unexpected times and/or unexpected addresses, possibly leading 
>>> to data
>>> +  leakage or corruption. The feature on its own provides a basic 
>>> level
>>> +  of protection against the DMA overwriting buffer contents at
>>> +  unexpected times. However, to protect against general data 
>>> leakage and
>>> +  system memory corruption, the system needs to provide way to 
>>> lock down
>>> +  the memory access, e.g., MPU.
>>
>> As far as I can tell, these pools work with both static allocations (which
>> seem to match your use-case where firmware has preconfigured the DMA ranges)
>> but also with dynamic allocations where a 'size' property is present instead
>> of the 'reg' property and the kernel is responsible for allocating the
>> reservation during boot. Am I right and, if so, is that deliberate?
> 
> I believe so. I'm not keen on having size only reservations in DT.
> Yes, we allowed that already, but that's back from the days of needing
> large CMA carveouts to be reserved early in boot. I've read that the
> kernel is much better now at contiguous allocations, so do we really
> need this in DT anymore?

I would say yes, there can be a number of times where you want to semi
statically partition your physical memory and their reserved regions. Be
it to pack everything together under the same protection rules or
because you need to allocate memory from a particular address range in
say a non-uniform memory controller architecture where address windows
have different scheduling algorithms.
-- 
Florian


Re: [PATCH RESEND v6 00/10] dt-bindings: usb: Harmonize xHCI/EHCI/OHCI/DWC3 nodes name

2021-02-10 Thread Florian Fainelli
On 2/10/21 9:28 AM, Serge Semin wrote:
> As the subject states this series is an attempt to harmonize the xHCI,
> EHCI, OHCI and DWC USB3 DT nodes with the DT schema introduced in the
> framework of the patchset [1].
> 
> Firstly as Krzysztof suggested we've deprecated a support of DWC USB3
> controllers with "synopsys,"-vendor prefix compatible string in favor of
> the ones with valid "snps,"-prefix. It's done in all the DTS files,
> which have been unfortunate to define such nodes.
> 
> Secondly we suggest to fix the snps,quirk-frame-length-adjustment property
> declaration in the Amlogic meson-g12-common.dtsi DTS file, since it has
> been erroneously declared as boolean while having uint32 type. Neil said
> it was ok to init that property with 0x20 value.
> 
> Thirdly the main part of the patchset concern fixing the xHCI, EHCI/OHCI
> and DWC USB3 DT nodes name as in accordance with their DT schema the
> corresponding node name is suppose to comply with the Generic USB HCD DT
> schema, which requires the USB nodes to have the name acceptable by the
> regexp: "^usb(@.*)?". Such requirement had been applicable even before we
> introduced the new DT schema in [1], but as we can see it hasn't been
> strictly implemented for a lot the DTS files. Since DT schema is now
> available the automated DTS validation shall make sure that the rule isn't
> violated.
> 
> Note most of these patches have been a part of the last three patches of
> [1]. But since there is no way to have them merged in in a combined
> manner, I had to move them to the dedicated series and split them up so to
> be accepted by the corresponding subsystem maintainers one-by-one.
> 
> [1] Link: 
> https://lore.kernel.org/linux-usb/20201014101402.18271-1-sergey.se...@baikalelectronics.ru/
> Changelog v1:
> - As Krzysztof suggested I've created a script which checked whether the
>   node names had been also updated in all the depended dts files. As a
>   result I found two more files which should have been also modified:
>   arch/arc/boot/dts/{axc003.dtsi,axc003_idu.dtsi}
> - Correct the USB DWC3 nodes name found in
>   arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} too.
> 
> Link: 
> https://lore.kernel.org/linux-usb/20201020115959.2658-1-sergey.se...@baikalelectronics.ru
> Changelog v2:
> - Drop the patch:
>   [PATCH 01/29] usb: dwc3: Discard synopsys,dwc3 compatibility string
>   and get back the one which marks the "synopsys,dwc3" compatible string
>   as deprecated into the DT schema related series.
> - Drop the patches:
>   [PATCH 03/29] arm: dts: am437x: Correct DWC USB3 compatible string
>   [PATCH 04/29] arm: dts: exynos: Correct DWC USB3 compatible string
>   [PATCH 07/29] arm: dts: bcm53x: Harmonize EHCI/OHCI DT nodes name
>   [PATCH 08/29] arm: dts: stm32: Harmonize EHCI/OHCI DT nodes name
>   [PATCH 16/29] arm: dts: bcm5301x: Harmonize xHCI DT nodes name
>   [PATCH 19/29] arm: dts: exynos: Harmonize DWC USB3 DT nodes name
>   [PATCH 21/29] arm: dts: ls1021a: Harmonize DWC USB3 DT nodes name
>   [PATCH 22/29] arm: dts: omap5: Harmonize DWC USB3 DT nodes name
>   [PATCH 24/29] arm64: dts: allwinner: h6: Harmonize DWC USB3 DT nodes name
>   [PATCH 26/29] arm64: dts: exynos: Harmonize DWC USB3 DT nodes name
>   [PATCH 27/29] arm64: dts: layerscape: Harmonize DWC USB3 DT nodes name
>   since they have been applied to the corresponding maintainers repos.
> - Fix drivers/usb/dwc3/dwc3-qcom.c to be looking for the "usb@"-prefixed
>   sub-node and falling back to the "dwc3@"-prefixed one on failure.
> 
> Link: 
> https://lore.kernel.org/linux-usb/2020091552.15593-1-sergey.se...@baikalelectronics.ru
> Changelog v3:
> - Drop the patches:
>   [PATCH v2 04/18] arm: dts: hisi-x5hd2: Harmonize EHCI/OHCI DT nodes name
>   [PATCH v2 06/18] arm64: dts: hisi: Harmonize EHCI/OHCI DT nodes name
>   [PATCH v2 07/18] mips: dts: jz47x: Harmonize EHCI/OHCI DT nodes name
>   [PATCH v2 08/18] mips: dts: sead3: Harmonize EHCI/OHCI DT nodes name
>   [PATCH v2 09/18] mips: dts: ralink: mt7628a: Harmonize EHCI/OHCI DT nodes 
> name
>   [PATCH v2 11/18] arm64: dts: marvell: cp11x: Harmonize xHCI DT nodes name
>   [PATCH v2 12/18] arm: dts: marvell: armada-375: Harmonize DWC USB3 DT nodes 
> name
>   [PATCH v2 16/18] arm64: dts: hi3660: Harmonize DWC USB3 DT nodes name
>   since they have been applied to the corresponding maintainers repos.
> 
> Link: 
> https://lore.kernel.org/linux-usb/20201205155621.3045-1-sergey.se...@baikalelectronics.ru
> Changelog v4:
> - Just resend.
> 
> Link: 
> https://lore.kernel.org/linux-usb/20201210091756.18057-1-sergey.se...@baikalelectronics.ru/
> Changelog v5:
> - Drop the patch:
>   [PATCH v4 02/10] arm64: dts: amlogic: meson-g12: Set FL-adj property value
>   since it has been applied to the corresponding maintainers repos.
> - Get back the patch:
>   [PATCH 21/29] arm: dts: ls1021a: Harmonize DWC USB3 DT nodes name
>   as it has been missing in the kernel 5.11-rc7
> - Rebase onto the kernel 5.11-rc7
> 
> Link: 

Re: [PATCH] powerpc/akebono: Fix unmet dependency errors

2021-01-31 Thread Florian Fainelli



On 1/31/2021 5:25 PM, Michael Ellerman wrote:
> The AKEBONO config has various selects under it, including some with
> user-selectable dependencies, which means those dependencies can be
> disabled. This leads to warnings from Kconfig.
> 
> This can be seen with eg:
> 
>   $ make allnoconfig
>   $ ./scripts/config --file build~/.config -k -e CONFIG_44x -k -e 
> CONFIG_PPC_47x -e CONFIG_AKEBONO
>   $ make olddefconfig
> 
>   WARNING: unmet direct dependencies detected for ATA
> Depends on [n]: HAS_IOMEM [=y] && BLOCK [=n]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
> 
>   WARNING: unmet direct dependencies detected for NETDEVICES
> Depends on [n]: NET [=n]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
> 
>   WARNING: unmet direct dependencies detected for ETHERNET
> Depends on [n]: NETDEVICES [=y] && NET [=n]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
> 
>   WARNING: unmet direct dependencies detected for MMC_SDHCI
> Depends on [n]: MMC [=n] && HAS_DMA [=y]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
> 
>   WARNING: unmet direct dependencies detected for MMC_SDHCI_PLTFM
> Depends on [n]: MMC [=n] && MMC_SDHCI [=y]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
> 
> The problem is that AKEBONO is using select to enable things that are
> not true dependencies, but rather things you probably want enabled in
> an AKEBONO kernel. That is what a defconfig is for.
> 
> So drop those selects and instead move those symbols into the
> defconfig. This fixes all the kconfig warnings, and the result of make
> 44x/akebono_defconfig is the same before and after the patch.
> 
> Reported-by: Yury Norov 
> Reported-by: Randy Dunlap 
> Reported-by: Florian Fainelli 
> Signed-off-by: Michael Ellerman 

Acked-by: Florian Fainelli 

Much better than my patch, thanks!
-- 
Florian


[PATCH 1/2] powerpc/Kconfig: Fix unmet direct dependency on NET

2021-01-30 Thread Florian Fainelli
The kbuild test robot was able to generate a configuration where
ETHERNET and NETDEVICES was selected by the Akenobo platform but not
NET, which resulted in various build failures and these Kconfig
warnings:

WARNING: unmet direct dependencies detected for NETDEVICES
  Depends on [n]: NET [=n]
  Selected by [y]:
  - AKEBONO [=y] && PPC_47x [=y]

WARNING: unmet direct dependencies detected for ETHERNET
  Depends on [n]: NETDEVICES [=y] && NET [=n]
  Selected by [y]:
  - AKEBONO [=y] && PPC_47x [=y]

Reported-by: kernel test robot 
Signed-off-by: Florian Fainelli 
---
 arch/powerpc/platforms/44x/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/44x/Kconfig 
b/arch/powerpc/platforms/44x/Kconfig
index 78ac6d67a935..68bd647c878f 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -206,6 +206,7 @@ config AKEBONO
select PPC4xx_HSTA_MSI
select I2C
select I2C_IBM_IIC
+   select NET
select NETDEVICES
select ETHERNET
select NET_VENDOR_IBM
-- 
2.25.1



Re: [RFC PATCH v3 6/6] of: Add plumbing for restricted DMA pool

2021-01-14 Thread Florian Fainelli
On 1/14/21 1:08 AM, Claire Chang wrote:
> On Wed, Jan 13, 2021 at 7:48 AM Florian Fainelli  wrote:
>>
>> On 1/5/21 7:41 PM, Claire Chang wrote:
>>> If a device is not behind an IOMMU, we look up the device node and set
>>> up the restricted DMA when the restricted-dma-pool is presented.
>>>
>>> Signed-off-by: Claire Chang 
>>> ---
>>
>> [snip]
>>
>>> +int of_dma_set_restricted_buffer(struct device *dev)
>>> +{
>>> + struct device_node *node;
>>> + int count, i;
>>> +
>>> + if (!dev->of_node)
>>> + return 0;
>>> +
>>> + count = of_property_count_elems_of_size(dev->of_node, "memory-region",
>>> + sizeof(phandle));
>>
>> You could have an early check for count < 0, along with an error
>> message, if that is deemed useful.
>>
>>> + for (i = 0; i < count; i++) {
>>> + node = of_parse_phandle(dev->of_node, "memory-region", i);
>>> + if (of_device_is_compatible(node, "restricted-dma-pool"))
>>
>> And you may want to add here an of_device_is_available(node). A platform
>> that provides the Device Tree firmware and try to support multiple
>> different SoCs may try to determine if an IOMMU is present, and if it
>> is, it could be marking the restriced-dma-pool region with a 'status =
>> "disabled"' property, or any variant of that scheme.
> 
> This function is called only when there is no IOMMU present (check in
> drivers/of/device.c). I can still add of_device_is_available(node)
> here if you think it's helpful.

I believe it is, since boot loader can have a shared Device Tree blob
skeleton and do various adaptations based on the chip (that's what we
do) and adding a status property is much simpler than insertion new
nodes are run time.

> 
>>
>>> + return of_reserved_mem_device_init_by_idx(
>>> + dev, dev->of_node, i);
>>
>> This does not seem to be supporting more than one memory region, did not
>> you want something like instead:
>>
>> ret = of_reserved_mem_device_init_by_idx(...);
>> if (ret)
>> return ret;
>>
> 
> Yes. This implement only supports one restriced-dma-pool memory region
> with the assumption that there is only one memory region with the
> compatible string, restricted-dma-pool, in the dts. IIUC, it's similar
> to shared-dma-pool.

Then if here is such a known limitation it should be both documented and
enforced here, you shouldn ot be iterating over all of the phandles that
you find, stop at the first one and issue a warning if count > 1?
-- 
Florian


Re: [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-13 Thread Florian Fainelli
On 1/13/21 7:27 AM, Robin Murphy wrote:
> On 2021-01-13 13:59, Nicolas Saenz Julienne wrote:
>> Hi All,
>>
>> On Tue, 2021-01-12 at 16:03 -0800, Florian Fainelli wrote:
>>> On 1/5/21 7:41 PM, Claire Chang wrote:
>>>> Add the initialization function to create restricted DMA pools from
>>>> matching reserved-memory nodes in the device tree.
>>>>
>>>> Signed-off-by: Claire Chang 
>>>> ---
>>>>   include/linux/device.h  |   4 ++
>>>>   include/linux/swiotlb.h |   7 +-
>>>>   kernel/dma/Kconfig  |   1 +
>>>>   kernel/dma/swiotlb.c    | 144
>>>> ++--
>>>>   4 files changed, 131 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/include/linux/device.h b/include/linux/device.h
>>>> index 89bb8b84173e..ca6f71ec8871 100644
>>>> --- a/include/linux/device.h
>>>> +++ b/include/linux/device.h
>>>> @@ -413,6 +413,7 @@ struct dev_links_info {
>>>>    * @dma_pools:    Dma pools (if dma'ble device).
>>>>    * @dma_mem:    Internal for coherent mem override.
>>>>    * @cma_area:    Contiguous memory area for dma allocations
>>>> + * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override.
>>>>    * @archdata:    For arch-specific additions.
>>>>    * @of_node:    Associated device tree node.
>>>>    * @fwnode:    Associated device node supplied by platform firmware.
>>>> @@ -515,6 +516,9 @@ struct device {
>>>>   #ifdef CONFIG_DMA_CMA
>>>>   struct cma *cma_area;    /* contiguous memory area for dma
>>>>  allocations */
>>>> +#endif
>>>> +#ifdef CONFIG_SWIOTLB
>>>> +    struct io_tlb_mem    *dma_io_tlb_mem;
>>>>   #endif
>>>>   /* arch specific additions */
>>>>   struct dev_archdata    archdata;
>>>> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
>>>> index dd8eb57cbb8f..a1bbd775 100644
>>>> --- a/include/linux/swiotlb.h
>>>> +++ b/include/linux/swiotlb.h
>>>> @@ -76,12 +76,13 @@ extern enum swiotlb_force swiotlb_force;
>>>>    *
>>>>    * @start:    The start address of the swiotlb memory pool. Used
>>>> to do a quick
>>>>    *    range check to see if the memory was in fact allocated
>>>> by this
>>>> - *    API.
>>>> + *    API. For restricted DMA pool, this is device tree
>>>> adjustable.
>>>
>>> Maybe write it as this is "firmware adjustable" such that when/if ACPI
>>> needs something like this, the description does not need updating.
> 
> TBH I really don't think this needs calling out at all. Even in the
> regular case, the details of exactly how and where the pool is allocated
> are beyond the scope of this code - architectures already have several
> ways to control that and make their own decisions.
> 
>>>
>>> [snip]
>>>
>>>> +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>>>> +    struct device *dev)
>>>> +{
>>>> +    struct io_tlb_mem *mem = rmem->priv;
>>>> +    int ret;
>>>> +
>>>> +    if (dev->dma_io_tlb_mem)
>>>> +    return -EBUSY;
>>>> +
>>>> +    if (!mem) {
>>>> +    mem = kzalloc(sizeof(*mem), GFP_KERNEL);
>>>> +    if (!mem)
>>>> +    return -ENOMEM;
>>>> +
>>>> +    if (!memremap(rmem->base, rmem->size, MEMREMAP_WB)) {
>>>
>>> MEMREMAP_WB sounds appropriate as a default.
>>
>> As per the binding 'no-map' has to be disabled here. So AFAIU, this
>> memory will
>> be part of the linear mapping. Is this really needed then?
> 
> More than that, I'd assume that we *have* to use the linear/direct map
> address rather than anything that has any possibility of being a vmalloc
> remap, otherwise we can no longer safely rely on
> phys_to_dma/dma_to_phys, no?

I believe you are right, which means that if we want to make use of the
restricted DMA pool on a 32-bit architecture (and we do, at least, I do)
we should probably add some error checking/warning to ensure the
restricted DMA pool falls within the linear map.
-- 
Florian


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-12 Thread Florian Fainelli



On 1/12/2021 8:25 PM, Tomasz Figa wrote:
> On Wed, Jan 13, 2021 at 12:56 PM Florian Fainelli  
> wrote:
>>
>>
>>
>> On 1/12/2021 6:29 PM, Tomasz Figa wrote:
>>> Hi Florian,
>>>
>>> On Wed, Jan 13, 2021 at 3:01 AM Florian Fainelli  
>>> wrote:
>>>>
>>>> On 1/11/21 11:48 PM, Claire Chang wrote:
>>>>> On Fri, Jan 8, 2021 at 1:59 AM Florian Fainelli  
>>>>> wrote:
>>>>>>
>>>>>> On 1/7/21 9:42 AM, Claire Chang wrote:
>>>>>>
>>>>>>>> Can you explain how ATF gets involved and to what extent it does help,
>>>>>>>> besides enforcing a secure region from the ARM CPU's perpsective? Does
>>>>>>>> the PCIe root complex not have an IOMMU but can somehow be denied 
>>>>>>>> access
>>>>>>>> to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
>>>>>>>> still some sort of basic protection that the HW enforces, right?
>>>>>>>
>>>>>>> We need the ATF support for memory MPU (memory protection unit).
>>>>>>> Restricted DMA (with reserved-memory in dts) makes sure the predefined 
>>>>>>> memory
>>>>>>> region is for PCIe DMA only, but we still need MPU to locks down PCIe 
>>>>>>> access to
>>>>>>> that specific regions.
>>>>>>
>>>>>> OK so you do have a protection unit of some sort to enforce which region
>>>>>> in DRAM the PCIE bridge is allowed to access, that makes sense,
>>>>>> otherwise the restricted DMA region would only be a hint but nothing you
>>>>>> can really enforce. This is almost entirely analogous to our systems 
>>>>>> then.
>>>>>
>>>>> Here is the example of setting the MPU:
>>>>> https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132
>>>>>
>>>>>>
>>>>>> There may be some value in standardizing on an ARM SMCCC call then since
>>>>>> you already support two different SoC vendors.
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On Broadcom STB SoCs we have had something similar for a while however
>>>>>>>> and while we don't have an IOMMU for the PCIe bridge, we do have a a
>>>>>>>> basic protection mechanism whereby we can configure a region in DRAM to
>>>>>>>> be PCIe read/write and CPU read/write which then gets used as the PCIe
>>>>>>>> inbound region for the PCIe EP. By default the PCIe bridge is not
>>>>>>>> allowed access to DRAM so we must call into a security agent to allow
>>>>>>>> the PCIe bridge to access the designated DRAM region.
>>>>>>>>
>>>>>>>> We have done this using a private CMA area region assigned via Device
>>>>>>>> Tree, assigned with a and requiring the PCIe EP driver to use
>>>>>>>> dma_alloc_from_contiguous() in order to allocate from this device
>>>>>>>> private CMA area. The only drawback with that approach is that it
>>>>>>>> requires knowing how much memory you need up front for buffers and DMA
>>>>>>>> descriptors that the PCIe EP will need to process. The problem is that
>>>>>>>> it requires driver modifications and that does not scale over the 
>>>>>>>> number
>>>>>>>> of PCIe EP drivers, some we absolutely do not control, but there is no
>>>>>>>> need to bounce buffer. Your approach scales better across PCIe EP
>>>>>>>> drivers however it does require bounce buffering which could be a
>>>>>>>> performance hit.
>>>>>>>
>>>>>>> Only the streaming DMA (map/unmap) needs bounce buffering.
>>>>>>
>>>>>> True, and typically only on transmit since you don't really control
>>>>>> where the sk_buff are allocated from, right? On RX since you need to
>>>>>> hand buffer addresses to the WLAN chip prior to DMA, you can allocate
>>>>>> them from a pool that already falls within the restricted DMA region, 
>>>>>> right?
>>>>>>
>>>>>
>>

Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-12 Thread Florian Fainelli



On 1/12/2021 6:29 PM, Tomasz Figa wrote:
> Hi Florian,
> 
> On Wed, Jan 13, 2021 at 3:01 AM Florian Fainelli  wrote:
>>
>> On 1/11/21 11:48 PM, Claire Chang wrote:
>>> On Fri, Jan 8, 2021 at 1:59 AM Florian Fainelli  
>>> wrote:
>>>>
>>>> On 1/7/21 9:42 AM, Claire Chang wrote:
>>>>
>>>>>> Can you explain how ATF gets involved and to what extent it does help,
>>>>>> besides enforcing a secure region from the ARM CPU's perpsective? Does
>>>>>> the PCIe root complex not have an IOMMU but can somehow be denied access
>>>>>> to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
>>>>>> still some sort of basic protection that the HW enforces, right?
>>>>>
>>>>> We need the ATF support for memory MPU (memory protection unit).
>>>>> Restricted DMA (with reserved-memory in dts) makes sure the predefined 
>>>>> memory
>>>>> region is for PCIe DMA only, but we still need MPU to locks down PCIe 
>>>>> access to
>>>>> that specific regions.
>>>>
>>>> OK so you do have a protection unit of some sort to enforce which region
>>>> in DRAM the PCIE bridge is allowed to access, that makes sense,
>>>> otherwise the restricted DMA region would only be a hint but nothing you
>>>> can really enforce. This is almost entirely analogous to our systems then.
>>>
>>> Here is the example of setting the MPU:
>>> https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132
>>>
>>>>
>>>> There may be some value in standardizing on an ARM SMCCC call then since
>>>> you already support two different SoC vendors.
>>>>
>>>>>
>>>>>>
>>>>>> On Broadcom STB SoCs we have had something similar for a while however
>>>>>> and while we don't have an IOMMU for the PCIe bridge, we do have a a
>>>>>> basic protection mechanism whereby we can configure a region in DRAM to
>>>>>> be PCIe read/write and CPU read/write which then gets used as the PCIe
>>>>>> inbound region for the PCIe EP. By default the PCIe bridge is not
>>>>>> allowed access to DRAM so we must call into a security agent to allow
>>>>>> the PCIe bridge to access the designated DRAM region.
>>>>>>
>>>>>> We have done this using a private CMA area region assigned via Device
>>>>>> Tree, assigned with a and requiring the PCIe EP driver to use
>>>>>> dma_alloc_from_contiguous() in order to allocate from this device
>>>>>> private CMA area. The only drawback with that approach is that it
>>>>>> requires knowing how much memory you need up front for buffers and DMA
>>>>>> descriptors that the PCIe EP will need to process. The problem is that
>>>>>> it requires driver modifications and that does not scale over the number
>>>>>> of PCIe EP drivers, some we absolutely do not control, but there is no
>>>>>> need to bounce buffer. Your approach scales better across PCIe EP
>>>>>> drivers however it does require bounce buffering which could be a
>>>>>> performance hit.
>>>>>
>>>>> Only the streaming DMA (map/unmap) needs bounce buffering.
>>>>
>>>> True, and typically only on transmit since you don't really control
>>>> where the sk_buff are allocated from, right? On RX since you need to
>>>> hand buffer addresses to the WLAN chip prior to DMA, you can allocate
>>>> them from a pool that already falls within the restricted DMA region, 
>>>> right?
>>>>
>>>
>>> Right, but applying bounce buffering to RX will make it more secure.
>>> The device won't be able to modify the content after unmap. Just like what
>>> iommu_unmap does.
>>
>> Sure, however the goals of using bounce buffering equally applies to RX
>> and TX in that this is the only layer sitting between a stack (block,
>> networking, USB, etc.) and the underlying device driver that scales well
>> in order to massage a dma_addr_t to be within a particular physical range.
>>
>> There is however room for improvement if the drivers are willing to
>> change their buffer allocation strategy. When you receive Wi-Fi frames
>> you need to allocate buffers for the Wi-Fi device to DMA into, and that
>> happens ahead of the DMA transfers by the Wi-Fi device. At buffer
>> allocation time you could very well allocate these frames from the
>> restricted DMA region without having to bounce buffer them since the
>> host CPU is in control over where and when to DMA into.
>>
> 
> That is, however, still a trade-off between saving that one copy and
> protection from the DMA tampering with the packet contents when the
> kernel is reading them. Notice how the copy effectively makes a
> snapshot of the contents, guaranteeing that the kernel has a
> consistent view of the packet, which is not true if the DMA could
> modify the buffer contents in the middle of CPU accesses.

I would say that the window just became so much narrower for the PCIe
end-point to overwrite contents with the copy because it would have to
happen within the dma_unmap_{page,single} time and before the copy is
finished to the bounce buffer.
-- 
Florian


Re: [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-12 Thread Florian Fainelli
On 1/5/21 7:41 PM, Claire Chang wrote:
> Add the initialization function to create restricted DMA pools from
> matching reserved-memory nodes in the device tree.
> 
> Signed-off-by: Claire Chang 
> ---
>  include/linux/device.h  |   4 ++
>  include/linux/swiotlb.h |   7 +-
>  kernel/dma/Kconfig  |   1 +
>  kernel/dma/swiotlb.c| 144 ++--
>  4 files changed, 131 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 89bb8b84173e..ca6f71ec8871 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -413,6 +413,7 @@ struct dev_links_info {
>   * @dma_pools:   Dma pools (if dma'ble device).
>   * @dma_mem: Internal for coherent mem override.
>   * @cma_area:Contiguous memory area for dma allocations
> + * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override.
>   * @archdata:For arch-specific additions.
>   * @of_node: Associated device tree node.
>   * @fwnode:  Associated device node supplied by platform firmware.
> @@ -515,6 +516,9 @@ struct device {
>  #ifdef CONFIG_DMA_CMA
>   struct cma *cma_area;   /* contiguous memory area for dma
>  allocations */
> +#endif
> +#ifdef CONFIG_SWIOTLB
> + struct io_tlb_mem   *dma_io_tlb_mem;
>  #endif
>   /* arch specific additions */
>   struct dev_archdata archdata;
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index dd8eb57cbb8f..a1bbd775 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -76,12 +76,13 @@ extern enum swiotlb_force swiotlb_force;
>   *
>   * @start:   The start address of the swiotlb memory pool. Used to do a quick
>   *   range check to see if the memory was in fact allocated by this
> - *   API.
> + *   API. For restricted DMA pool, this is device tree adjustable.

Maybe write it as this is "firmware adjustable" such that when/if ACPI
needs something like this, the description does not need updating.

[snip]

> +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> + struct device *dev)
> +{
> + struct io_tlb_mem *mem = rmem->priv;
> + int ret;
> +
> + if (dev->dma_io_tlb_mem)
> + return -EBUSY;
> +
> + if (!mem) {
> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> + if (!mem)
> + return -ENOMEM;
> +
> + if (!memremap(rmem->base, rmem->size, MEMREMAP_WB)) {

MEMREMAP_WB sounds appropriate as a default.
Documentation/devicetree/bindings/reserved-memory/ramoops.txt does
define an "unbuffered" property which in premise could be applied to the
generic reserved memory binding as well and that we may have to be
honoring here, if we were to make it more generic. Oh well, this does
not need to be addressed right now I guess.
-- 
Florian


Re: [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-12 Thread Florian Fainelli
On 1/7/21 1:19 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 07, 2021 at 10:09:14AM -0800, Florian Fainelli wrote:
>> On 1/7/21 9:57 AM, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Jan 08, 2021 at 01:39:18AM +0800, Claire Chang wrote:
>>>> Hi Greg and Konrad,
>>>>
>>>> This change is intended to be non-arch specific. Any arch that lacks DMA 
>>>> access
>>>> control and has devices not behind an IOMMU can make use of it. Could you 
>>>> share
>>>> why you think this should be arch specific?
>>>
>>> The idea behind non-arch specific code is it to be generic. The devicetree
>>> is specific to PowerPC, Sparc, and ARM, and not to x86 - hence it should
>>> be in arch specific code.
>>
>> In premise the same code could be used with an ACPI enabled system with
>> an appropriate service to identify the restricted DMA regions and unlock
>> them.
> 
> Which this patchset is not.

ACPI is not included, but the comment about Device Tree being specific
to PowerPC, SPARC and ARM is x86 is not quite correct. There is an
architecture specific part to obtaining where the Device Tree lives in
memory, but the implementation itself is architecture agnostic (with
some early SPARC/OpenFirmware shenanigans), and x86 does, or rather did
support Device Tree to a very small extent with the CE4100 platform.

Would you prefer that an swiotlb_of.c file be created instead or
something along those lines to better encapsulate where the OF specific
code lives?

> 
>>
>> More than 1 architecture requiring this function (ARM and ARM64 are the
>> two I can think of needing this immediately) sort of calls for making
>> the code architecture agnostic since past 2, you need something that scales.
> 
> I believe the use-case is for ARM64 at this moment.

For the platforms that Claire uses, certainly for the ones we use, ARM
and ARM64 are in scope.
-- 
Florian


Re: [RFC PATCH v3 6/6] of: Add plumbing for restricted DMA pool

2021-01-12 Thread Florian Fainelli
On 1/5/21 7:41 PM, Claire Chang wrote:
> If a device is not behind an IOMMU, we look up the device node and set
> up the restricted DMA when the restricted-dma-pool is presented.
> 
> Signed-off-by: Claire Chang 
> ---

[snip]

> +int of_dma_set_restricted_buffer(struct device *dev)
> +{
> + struct device_node *node;
> + int count, i;
> +
> + if (!dev->of_node)
> + return 0;
> +
> + count = of_property_count_elems_of_size(dev->of_node, "memory-region",
> + sizeof(phandle));

You could have an early check for count < 0, along with an error
message, if that is deemed useful.

> + for (i = 0; i < count; i++) {
> + node = of_parse_phandle(dev->of_node, "memory-region", i);
> + if (of_device_is_compatible(node, "restricted-dma-pool"))

And you may want to add here an of_device_is_available(node). A platform
that provides the Device Tree firmware and try to support multiple
different SoCs may try to determine if an IOMMU is present, and if it
is, it could be marking the restriced-dma-pool region with a 'status =
"disabled"' property, or any variant of that scheme.

> + return of_reserved_mem_device_init_by_idx(
> + dev, dev->of_node, i);

This does not seem to be supporting more than one memory region, did not
you want something like instead:

ret = of_reserved_mem_device_init_by_idx(...);
if (ret)
return ret;

> + }
> +
> + return 0;
> +}
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index aedfaaafd3e7..e2c7409956ab 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -182,6 +182,10 @@ int of_dma_configure_id(struct device *dev, struct 
> device_node *np,
>   arch_setup_dma_ops(dev, dma_start, size, iommu, coherent);
>  
>   dev->dma_range_map = map;
> +
> + if (!iommu)
> + return of_dma_set_restricted_buffer(dev);
> +
>   return 0;
>  }
>  EXPORT_SYMBOL_GPL(of_dma_configure_id);
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index d9e6a324de0a..28a2dfa197ba 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -161,12 +161,17 @@ struct bus_dma_region;
>  #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_HAS_DMA)
>  int of_dma_get_range(struct device_node *np,
>   const struct bus_dma_region **map);
> +int of_dma_set_restricted_buffer(struct device *dev);
>  #else
>  static inline int of_dma_get_range(struct device_node *np,
>   const struct bus_dma_region **map)
>  {
>   return -ENODEV;
>  }
> +static inline int of_dma_get_restricted_buffer(struct device *dev)
> +{
> + return -ENODEV;
> +}
>  #endif
>  
>  #endif /* _LINUX_OF_PRIVATE_H */
> 


-- 
Florian


Re: [RFC PATCH v3 4/6] swiotlb: Add restricted DMA alloc/free support.

2021-01-12 Thread Florian Fainelli
On 1/5/21 7:41 PM, Claire Chang wrote:
> Add the functions, swiotlb_alloc and swiotlb_free to support the
> memory allocation from restricted DMA pool.
> 
> Signed-off-by: Claire Chang 
> ---

[snip]

> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 30ccbc08e229..126e9b3354d6 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -137,6 +137,11 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>   void *ret;
>   int err;
>  
> +#ifdef CONFIG_SWIOTLB
> + if (unlikely(dev->dma_io_tlb_mem))
> + return swiotlb_alloc(dev, size, dma_handle, attrs);
> +#endif

While this is potentially a hot path, I am not sure of the unkikely is
warranted, maybe best left as a plain conditional.
-- 
Florian


Re: [RFC PATCH v3 3/6] swiotlb: Use restricted DMA pool if available

2021-01-12 Thread Florian Fainelli
On 1/5/21 7:41 PM, Claire Chang wrote:
> Regardless of swiotlb setting, the restricted DMA pool is preferred if
> available.
> 
> The restricted DMA pools provide a basic level of protection against
> the DMA overwriting buffer contents at unexpected times. However, to
> protect against general data leakage and system memory corruption, the
> system needs to provide a way to restrict the DMA to a predefined memory
> region.
> 
> Signed-off-by: Claire Chang 

You could probably split this patch into two:

- one that introduces the get_io_tlb_mem() getter, updates all callers
of is_swiotlb_buffer() to gain a 'struct device' argument
- another one that does add support for a non-default swiotlb pool and
adds dev->dma_io_tlb_mem

Other than that, LGTM!
-- 
Florian


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-12 Thread Florian Fainelli
On 1/11/21 11:48 PM, Claire Chang wrote:
> On Fri, Jan 8, 2021 at 1:59 AM Florian Fainelli  wrote:
>>
>> On 1/7/21 9:42 AM, Claire Chang wrote:
>>
>>>> Can you explain how ATF gets involved and to what extent it does help,
>>>> besides enforcing a secure region from the ARM CPU's perpsective? Does
>>>> the PCIe root complex not have an IOMMU but can somehow be denied access
>>>> to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
>>>> still some sort of basic protection that the HW enforces, right?
>>>
>>> We need the ATF support for memory MPU (memory protection unit).
>>> Restricted DMA (with reserved-memory in dts) makes sure the predefined 
>>> memory
>>> region is for PCIe DMA only, but we still need MPU to locks down PCIe 
>>> access to
>>> that specific regions.
>>
>> OK so you do have a protection unit of some sort to enforce which region
>> in DRAM the PCIE bridge is allowed to access, that makes sense,
>> otherwise the restricted DMA region would only be a hint but nothing you
>> can really enforce. This is almost entirely analogous to our systems then.
> 
> Here is the example of setting the MPU:
> https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132
> 
>>
>> There may be some value in standardizing on an ARM SMCCC call then since
>> you already support two different SoC vendors.
>>
>>>
>>>>
>>>> On Broadcom STB SoCs we have had something similar for a while however
>>>> and while we don't have an IOMMU for the PCIe bridge, we do have a a
>>>> basic protection mechanism whereby we can configure a region in DRAM to
>>>> be PCIe read/write and CPU read/write which then gets used as the PCIe
>>>> inbound region for the PCIe EP. By default the PCIe bridge is not
>>>> allowed access to DRAM so we must call into a security agent to allow
>>>> the PCIe bridge to access the designated DRAM region.
>>>>
>>>> We have done this using a private CMA area region assigned via Device
>>>> Tree, assigned with a and requiring the PCIe EP driver to use
>>>> dma_alloc_from_contiguous() in order to allocate from this device
>>>> private CMA area. The only drawback with that approach is that it
>>>> requires knowing how much memory you need up front for buffers and DMA
>>>> descriptors that the PCIe EP will need to process. The problem is that
>>>> it requires driver modifications and that does not scale over the number
>>>> of PCIe EP drivers, some we absolutely do not control, but there is no
>>>> need to bounce buffer. Your approach scales better across PCIe EP
>>>> drivers however it does require bounce buffering which could be a
>>>> performance hit.
>>>
>>> Only the streaming DMA (map/unmap) needs bounce buffering.
>>
>> True, and typically only on transmit since you don't really control
>> where the sk_buff are allocated from, right? On RX since you need to
>> hand buffer addresses to the WLAN chip prior to DMA, you can allocate
>> them from a pool that already falls within the restricted DMA region, right?
>>
> 
> Right, but applying bounce buffering to RX will make it more secure.
> The device won't be able to modify the content after unmap. Just like what
> iommu_unmap does.

Sure, however the goals of using bounce buffering equally applies to RX
and TX in that this is the only layer sitting between a stack (block,
networking, USB, etc.) and the underlying device driver that scales well
in order to massage a dma_addr_t to be within a particular physical range.

There is however room for improvement if the drivers are willing to
change their buffer allocation strategy. When you receive Wi-Fi frames
you need to allocate buffers for the Wi-Fi device to DMA into, and that
happens ahead of the DMA transfers by the Wi-Fi device. At buffer
allocation time you could very well allocate these frames from the
restricted DMA region without having to bounce buffer them since the
host CPU is in control over where and when to DMA into.

The issue is that each network driver may implement its own buffer
allocation strategy, some may simply call netdev_alloc_skb() which gives
zero control over where the buffer comes from unless you play tricks
with NUMA node allocations and somehow declare that your restricted DMA
region is a different NUMA node. If the driver allocates pages and then
attaches a SKB to that page using build_skb(), then you have much more
control over where that page comes from, and this is where using a
device

Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool

2021-01-07 Thread Florian Fainelli
On 1/7/21 10:00 AM, Konrad Rzeszutek Wilk wrote:
>>>
>>>
>>>  - Nothing stops the physical device from bypassing the SWIOTLB buffer.
>>>That is if an errant device screwed up the length or DMA address, the
>>>SWIOTLB would gladly do what the device told it do?
>>
>> So the system needs to provide a way to lock down the memory access, e.g. 
>> MPU.
> 
> OK! Would it be prudent to have this in the description above perhaps?

Yes this is something that must be documented as a requirement for the
restricted DMA pool users, otherwise attempting to do restricted DMA
pool is no different than say, using a device private CMA region.
Without the enforcement, this is just a best effort.
-- 
Florian


Re: [RFC PATCH v3 2/6] swiotlb: Add restricted DMA pool

2021-01-07 Thread Florian Fainelli
On 1/7/21 9:57 AM, Konrad Rzeszutek Wilk wrote:
> On Fri, Jan 08, 2021 at 01:39:18AM +0800, Claire Chang wrote:
>> Hi Greg and Konrad,
>>
>> This change is intended to be non-arch specific. Any arch that lacks DMA 
>> access
>> control and has devices not behind an IOMMU can make use of it. Could you 
>> share
>> why you think this should be arch specific?
> 
> The idea behind non-arch specific code is it to be generic. The devicetree
> is specific to PowerPC, Sparc, and ARM, and not to x86 - hence it should
> be in arch specific code.

In premise the same code could be used with an ACPI enabled system with
an appropriate service to identify the restricted DMA regions and unlock
them.

More than 1 architecture requiring this function (ARM and ARM64 are the
two I can think of needing this immediately) sort of calls for making
the code architecture agnostic since past 2, you need something that scales.

There is already code today under kernel/dma/contiguous.c that is only
activated on a CONFIG_OF=y && CONFIG_OF_RESERVED_MEM=y system, this is
no different.
-- 
Florian


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-07 Thread Florian Fainelli
On 1/7/21 9:42 AM, Claire Chang wrote:

>> Can you explain how ATF gets involved and to what extent it does help,
>> besides enforcing a secure region from the ARM CPU's perpsective? Does
>> the PCIe root complex not have an IOMMU but can somehow be denied access
>> to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
>> still some sort of basic protection that the HW enforces, right?
> 
> We need the ATF support for memory MPU (memory protection unit).
> Restricted DMA (with reserved-memory in dts) makes sure the predefined memory
> region is for PCIe DMA only, but we still need MPU to locks down PCIe access 
> to
> that specific regions.

OK so you do have a protection unit of some sort to enforce which region
in DRAM the PCIE bridge is allowed to access, that makes sense,
otherwise the restricted DMA region would only be a hint but nothing you
can really enforce. This is almost entirely analogous to our systems then.

There may be some value in standardizing on an ARM SMCCC call then since
you already support two different SoC vendors.

> 
>>
>> On Broadcom STB SoCs we have had something similar for a while however
>> and while we don't have an IOMMU for the PCIe bridge, we do have a a
>> basic protection mechanism whereby we can configure a region in DRAM to
>> be PCIe read/write and CPU read/write which then gets used as the PCIe
>> inbound region for the PCIe EP. By default the PCIe bridge is not
>> allowed access to DRAM so we must call into a security agent to allow
>> the PCIe bridge to access the designated DRAM region.
>>
>> We have done this using a private CMA area region assigned via Device
>> Tree, assigned with a and requiring the PCIe EP driver to use
>> dma_alloc_from_contiguous() in order to allocate from this device
>> private CMA area. The only drawback with that approach is that it
>> requires knowing how much memory you need up front for buffers and DMA
>> descriptors that the PCIe EP will need to process. The problem is that
>> it requires driver modifications and that does not scale over the number
>> of PCIe EP drivers, some we absolutely do not control, but there is no
>> need to bounce buffer. Your approach scales better across PCIe EP
>> drivers however it does require bounce buffering which could be a
>> performance hit.
> 
> Only the streaming DMA (map/unmap) needs bounce buffering.

True, and typically only on transmit since you don't really control
where the sk_buff are allocated from, right? On RX since you need to
hand buffer addresses to the WLAN chip prior to DMA, you can allocate
them from a pool that already falls within the restricted DMA region, right?

> I also added alloc/free support in this series
> (https://lore.kernel.org/patchwork/patch/1360995/), so dma_direct_alloc() will
> try to allocate memory from the predefined memory region.
> 
> As for the performance hit, it should be similar to the default swiotlb.
> Here are my experiment results. Both SoCs lack IOMMU for PCIe.
> 
> PCIe wifi vht80 throughput -
> 
>   MTK SoC  tcp_tx tcp_rxudp_tx   udp_rx
>   w/o Restricted DMA  244.1 134.66   312.56   350.79
>   w/ Restricted DMA246.95   136.59   363.21   351.99
> 
>   Rockchip SoC   tcp_tx tcp_rxudp_tx   udp_rx
>   w/o Restricted DMA  237.87   133.86   288.28   361.88
>   w/ Restricted DMA256.01   130.95   292.28   353.19

How come you get better throughput with restricted DMA? Is it because
doing DMA to/from a contiguous region allows for better grouping of
transactions from the DRAM controller's perspective somehow?

> 
> The CPU usage doesn't increase too much either.
> Although I didn't measure the CPU usage very precisely, it's ~3% with a single
> big core (Cortex-A72) and ~5% with a single small core (Cortex-A53).
> 
> Thanks!
> 
>>
>> Thanks!
>> --
>> Florian


-- 
Florian


Re: [RFC PATCH v3 0/6] Restricted DMA

2021-01-06 Thread Florian Fainelli
Hi,

First of all let me say that I am glad that someone is working on a
upstream solution for this issue, would appreciate if you could CC and
Jim Quinlan on subsequent submissions.

On 1/5/21 7:41 PM, Claire Chang wrote:
> This series implements mitigations for lack of DMA access control on
> systems without an IOMMU, which could result in the DMA accessing the
> system memory at unexpected times and/or unexpected addresses, possibly
> leading to data leakage or corruption.
> 
> For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is
> not behind an IOMMU. As PCI-e, by design, gives the device full access to
> system memory, a vulnerability in the Wi-Fi firmware could easily escalate
> to a full system exploit (remote wifi exploits: [1a], [1b] that shows a
> full chain of exploits; [2], [3]).
> 
> To mitigate the security concerns, we introduce restricted DMA. Restricted
> DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a
> specially allocated region and does memory allocation from the same region.
> The feature on its own provides a basic level of protection against the DMA
> overwriting buffer contents at unexpected times. However, to protect
> against general data leakage and system memory corruption, the system needs
> to provide a way to restrict the DMA to a predefined memory region (this is
> usually done at firmware level, e.g. in ATF on some ARM platforms).

Can you explain how ATF gets involved and to what extent it does help,
besides enforcing a secure region from the ARM CPU's perpsective? Does
the PCIe root complex not have an IOMMU but can somehow be denied access
to a region that is marked NS=0 in the ARM CPU's MMU? If so, that is
still some sort of basic protection that the HW enforces, right?

On Broadcom STB SoCs we have had something similar for a while however
and while we don't have an IOMMU for the PCIe bridge, we do have a a
basic protection mechanism whereby we can configure a region in DRAM to
be PCIe read/write and CPU read/write which then gets used as the PCIe
inbound region for the PCIe EP. By default the PCIe bridge is not
allowed access to DRAM so we must call into a security agent to allow
the PCIe bridge to access the designated DRAM region.

We have done this using a private CMA area region assigned via Device
Tree, assigned with a and requiring the PCIe EP driver to use
dma_alloc_from_contiguous() in order to allocate from this device
private CMA area. The only drawback with that approach is that it
requires knowing how much memory you need up front for buffers and DMA
descriptors that the PCIe EP will need to process. The problem is that
it requires driver modifications and that does not scale over the number
of PCIe EP drivers, some we absolutely do not control, but there is no
need to bounce buffer. Your approach scales better across PCIe EP
drivers however it does require bounce buffering which could be a
performance hit.

Thanks!
-- 
Florian


Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers

2020-12-06 Thread Florian Fainelli
+JimQ,

On 12/6/2020 12:16 PM, Krzysztof Wilczyński wrote:
> Hello Nicolas, Florian and Florian,
> 
> [...]
>> -/* Configuration space read/write support */
>> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
>> -{
>> -return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
>> -| ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
>> -| (busnr << PCIE_EXT_BUSNUM_SHIFT)
>> -| (reg & ~3);
>> -}
>> -
>>  static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int 
>> devfn,
>>  int where)
>>  {
>> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus 
>> *bus, unsigned int devfn,
>>  return PCI_SLOT(devfn) ? NULL : base + where;
>>  
>>  /* For devices, write to the config space index register */
>> -idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
>> +idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
>>  writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
>>  return base + PCIE_EXT_CFG_DATA + where;
>>  }
> [...]
> 
> Passing the hard-coded 0 as the "reg" argument here never actually did
> anything, thus the 32 bit alignment was never correctly enforced.
> 
> My question would be: should this be 32 bit aligned?  It seems like the
> intention was to perhaps make the alignment?  I am sadly not intimately
> familiar with his hardware, so I am not sure if there is something to
> fix here or not.
> 
> Also, I wonder whether it would be safe to pass the offset (the "where"
> variable) rather than hard-coded 0?
> 
> Thank you for help in advance!
> 
> Bjorn also asked the same question:
>   
> https://lore.kernel.org/linux-pci/20201120203428.GA272511@bjorn-Precision-5520/
> 
> Krzysztof
> 

-- 
Florian


Re: [PATCH 07/20] dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device

2020-10-15 Thread Florian Fainelli
On 10/14/20 3:13 AM, Serge Semin wrote:
> For some reason the "brcm,xhci-brcm-v2" compatible string has been missing
> in the original bindings file. Add it to the Generic xHCI Controllers DT
> schema since the controller driver expects it to be supported.
> 
> Signed-off-by: Serge Semin 

Acked-by: Florian Fainelli 
-- 
Florian


Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name

2020-10-15 Thread Florian Fainelli
On 10/14/20 3:14 AM, Serge Semin wrote:
> In accordance with the Generic EHCI/OHCI bindings the corresponding node
> name is suppose to comply with the Generic USB HCD DT schema, which
> requires the USB nodes to have the name acceptable by the regexp:
> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
> incompatible names.
> 
> Signed-off-by: Serge Semin 
> 
> ---
> 
> Please, test the patch out to make sure it doesn't brake the dependent DTS
> files. I did only a manual grepping of the possible nodes dependencies.
> ---

>  arch/arm/boot/dts/bcm5301x.dtsi| 4 ++--
>  arch/arm/boot/dts/bcm53573.dtsi| 4 ++--
Acked-by: Florian Fainelli 
-- 
Florian


Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name

2020-10-14 Thread Florian Fainelli
On 10/14/20 11:11 AM, Serge Semin wrote:
> On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote:
>> On 10/14/20 3:14 AM, Serge Semin wrote:
>>> In accordance with the Generic EHCI/OHCI bindings the corresponding node
>>> name is suppose to comply with the Generic USB HCD DT schema, which
>>> requires the USB nodes to have the name acceptable by the regexp:
>>> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
>>> incompatible names.
>>>
>>> Signed-off-by: Serge Semin 
>>>
>>> ---
>>>
>>> Please, test the patch out to make sure it doesn't brake the dependent DTS
>>> files. I did only a manual grepping of the possible nodes dependencies.
>>
> 
>> Not sure how you envisioned these change to be picked up, but you may
>> need to split these changes between ARM/ARM64, MIPS and PowerPC at
>> least. And within ARM/ARM64 you will most likely have to split according
>> to the various SoC maintainers.
> 
> Hmm, I don't really know how it's going to be done in this case, but there 
> must
> be a way to get the cross-platform patches picked up in general. For
> instance, see the patches like:
> 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread()
> 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS
> They touched the files from different files, but still have been merged in.

That situation is different, when a new facility is added and someone
has gone through the work of adding support for all architectures (or
nearly all of them), you want them to be merged in a way that limits
merge conflicts with other architecture changes.

Here you are fixing warnings, and each file you touch can clearly be
independently change from other files in the series without causing
merge conflicts. You are however creating the potential for merge
conflicts with other changes that the various SoC maintainers have
queued up.

> Maybe I should have copied these three patches to the 
> "linux-a...@vger.kernel.org"
> list or some other mailing list...

Maybe Rob can queue them through his device tree repository, with the
ack of the various SoC maintainers...
-- 
Florian


Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name

2020-10-14 Thread Florian Fainelli
On 10/14/20 3:14 AM, Serge Semin wrote:
> In accordance with the Generic EHCI/OHCI bindings the corresponding node
> name is suppose to comply with the Generic USB HCD DT schema, which
> requires the USB nodes to have the name acceptable by the regexp:
> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
> incompatible names.
> 
> Signed-off-by: Serge Semin 
> 
> ---
> 
> Please, test the patch out to make sure it doesn't brake the dependent DTS
> files. I did only a manual grepping of the possible nodes dependencies.

Not sure how you envisioned these change to be picked up, but you may
need to split these changes between ARM/ARM64, MIPS and PowerPC at
least. And within ARM/ARM64 you will most likely have to split according
to the various SoC maintainers.
-- 
Florian


Re: [PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers

2020-10-04 Thread Florian Fainelli




On 10/4/2020 5:38 PM, Krzysztof Wilczyński wrote:

Unify ECAM-related constants into a single set of standard constants
defining memory address shift values for the byte-level address that can
be used when accessing the PCI Express Configuration Space, and then
move native PCI Express controller drivers to use newly introduced
definitions retiring any driver-specific ones.

The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
PCI Express specification (see PCI Express Base Specification, Revision
5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
implement it the same way.  Most of the native PCI Express controller
drivers define their ECAM-related constants, many of these could be
shared, or use open-coded values when setting the .bus_shift field of
the struct pci_ecam_ops.

All of the newly added constants should remove ambiguity and reduce the
number of open-coded values, and also correlate more strongly with the
descriptions in the aforementioned specification (see Table 7-1
"Enhanced Configuration Address Mapping", p. 677).

There is no change to functionality.

Suggested-by: Bjorn Helgaas 
Signed-off-by: Krzysztof Wilczyński 
---


[snip]

  
-/* Configuration space read/write support */

-static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
-{
-   return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
-   | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
-   | (busnr << PCIE_EXT_BUSNUM_SHIFT)
-   | (reg & ~3);
-}
-
  static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int 
devfn,
int where)
  {
@@ -590,7 +578,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus 
*bus, unsigned int devfn,
return PCI_SLOT(devfn) ? NULL : base + where;
  
  	/* For devices, write to the config space index register */

-   idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
+   idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEVFN(devfn);


This appears to be correct, so:

Acked-by: Florian Fainelli 

however, I would have defined a couple of additional helper macros and do:

	idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEV(devfn) | 
PCIE_ECAM_FUN(devfn);


for clarity.

[snip]


+/*
+ * Memory address shift values for the byte-level address that
+ * can be used when accessing the PCI Express Configuration Space.
+ */
+
+/*
+ * Enhanced Configuration Access Mechanism (ECAM)
+ *
+ * See PCI Express Base Specification, Revision 5.0, Version 1.0,
+ * Section 7.2.2, Table 7-1, p. 677.
+ */
+#define PCIE_ECAM_BUS_SHIFT20 /* Bus Number */
+#define PCIE_ECAM_DEV_SHIFT15 /* Device Number */
+#define PCIE_ECAM_FUN_SHIFT12 /* Function Number */
+
+#define PCIE_ECAM_BUS(x)   (((x) & 0xff) << PCIE_ECAM_BUS_SHIFT)
+#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << PCIE_ECAM_FUN_SHIFT)


For instance, adding these two:

#define PCIE_ECAM_DEV(x)(((x) & 0x1f) << PCIE_ECAM_DEV_SHIFT)
#define PCIE_ECAM_FUN(x)(((x) & 0x7) << PCIE_ECAM_FUN_SHIFT)

may be clearer for use in drivers like pcie-brcmstb.c that used to treat 
the device function in terms of device and function (though it was 
called slot there).

--
Florian


Re: [RFC PATCH dpss_eth] Don't initialise ports with no PHY

2020-04-24 Thread Florian Fainelli



On 4/24/2020 3:29 PM, Darren Stevens wrote:
> Since cbb961ca271e ("Use random MAC address when none is given")
> Varisys Cyrus P5020 boards have been listing 5 ethernet ports instead of
> the 2 the board has.This is because we were preventing the adding of the
> unused ports by not suppling them a MAC address, which this patch now
> supplies.
> 
> Prevent them from appearing in the net devices list by checking for a
> 'status="disabled"' entry during probe and skipping the port if we find
> it. 
> 
> Signed-off-by: Darren Stevens 
> 
> ---
> 
>  drivers/net/ethernet/freescale/fman/mac.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c 
> b/drivers/net/ethernet/freescale/fman/mac.c
> index 43427c5..c9ed411 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -606,6 +606,7 @@ static int mac_probe(struct platform_device *_of_dev)
>   struct resource  res;
>   struct mac_priv_s   *priv;
>   const u8*mac_addr;
> + const char  *prop;
>   u32  val;
>   u8  fman_id;
>   phy_interface_t  phy_if;
> @@ -628,6 +629,16 @@ static int mac_probe(struct platform_device *_of_dev)
>   mac_dev->priv = priv;
>   priv->dev = dev;
>  
> + /* check for disabled devices and skip them, as now a missing
> +  * MAC address will be replaced with a Random one rather than
> +  * disabling the port
> +  */
> + prop = of_get_property(mac_node, "status", NULL);
> + if (prop && !strncmp(prop, "disabled", 8) {
> + err = -ENODEV;
> + goto _return
> + }

There is a sorter version: of_device_is_available(mac_node) which will
do the same thing.

> +
>   if (of_device_is_compatible(mac_node, "fsl,fman-dtsec")) {
>   setup_dtsec(mac_dev);
>   priv->internal_phy_node = of_parse_phandle(mac_node,
> 

-- 
Florian


Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

2020-01-21 Thread Florian Fainelli



On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
> Convert suitable drivers to use new helper phy_do_ioctl_running.
> 
> Signed-off-by: Heiner Kallweit 
The vast majority of drivers that you are converting use the following
convention:

- !netif_running -> return -EINVAL
- !dev->phydev -> return -ENODEV

so it may make sense to change the helper to accommodate the majority
here, not that I believe this is going to make much practical
difference, but if there were test cases that were specifically looking
for such an error code, they could be failing after this changeset.

For bgmac.c, bcmgenet.c and cpmac.c:

Acked-by: Florian Fainelli 

Whether you decide to spin another version or not.
-- 
Florian


Re: [PATCH] dma-mapping: treat dev->bus_dma_mask as a DMA limit

2019-11-13 Thread Florian Fainelli
On 11/13/19 12:34 PM, Robin Murphy wrote:
> On 13/11/2019 4:13 pm, Nicolas Saenz Julienne wrote:
>> Using a mask to represent bus DMA constraints has a set of limitations.
>> The biggest one being it can only hold a power of two (minus one). The
>> DMA mapping code is already aware of this and treats dev->bus_dma_mask
>> as a limit. This quirk is already used by some architectures although
>> still rare.
>>
>> With the introduction of the Raspberry Pi 4 we've found a new contender
>> for the use of bus DMA limits, as its PCIe bus can only address the
>> lower 3GB of memory (of a total of 4GB). This is impossible to represent
>> with a mask. To make things worse the device-tree code rounds non power
>> of two bus DMA limits to the next power of two, which is unacceptable in
>> this case.
>>
>> In the light of this, rename dev->bus_dma_mask to dev->bus_dma_limit all
>> over the tree and treat it as such. Note that dev->bus_dma_limit is
>> meant to contain the higher accesible DMA address.
> 
> Neat, you win a "why didn't I do it that way in the first place?" :)
> 
> Looking at it without all the history of previous attempts, this looks
> entirely reasonable, and definitely a step in the right direction.

And while you are changing those, would it make sense to not only rename
the structure member but introduce a getter and setter in order to ease
future work where this would no longer be a scalar?
-- 
Florian


Re: [PATCH -next 00/13] hwrng: use devm_platform_ioremap_resource() to simplify code

2019-10-16 Thread Florian Fainelli
On 10/16/19 3:46 AM, YueHaibing wrote:
> devm_platform_ioremap_resource() internally have platform_get_resource()
> and devm_ioremap_resource() in it. So instead of calling them separately
> use devm_platform_ioremap_resource() directly.

Did your coccinelle script not cover
drivers/char/hw_random/iproc-rng200.c somehow? Do you mind including it
as a separate patch?

Thanks

> 
> YueHaibing (13):
>   hwrng: atmel - use devm_platform_ioremap_resource() to simplify code
>   hwrng: bcm2835 - use devm_platform_ioremap_resource() to simplify code
>   hwrng: exynos - use devm_platform_ioremap_resource() to simplify code
>   hwrng: hisi - use devm_platform_ioremap_resource() to simplify code
>   hwrng: ks-sa - use devm_platform_ioremap_resource() to simplify code
>   hwrng: meson - use devm_platform_ioremap_resource() to simplify code
>   hwrng: npcm - use devm_platform_ioremap_resource() to simplify code
>   hwrng: omap - use devm_platform_ioremap_resource() to simplify code
>   hwrng: pasemi - use devm_platform_ioremap_resource() to simplify code
>   hwrng: pic32 - use devm_platform_ioremap_resource() to simplify code
>   hwrng: st - use devm_platform_ioremap_resource() to simplify code
>   hwrng: tx4939 - use devm_platform_ioremap_resource() to simplify code
>   hwrng: xgene - use devm_platform_ioremap_resource() to simplify code
> 
>  drivers/char/hw_random/atmel-rng.c   | 4 +---
>  drivers/char/hw_random/bcm2835-rng.c | 5 +
>  drivers/char/hw_random/exynos-trng.c | 4 +---
>  drivers/char/hw_random/hisi-rng.c| 4 +---
>  drivers/char/hw_random/ks-sa-rng.c   | 4 +---
>  drivers/char/hw_random/meson-rng.c   | 4 +---
>  drivers/char/hw_random/npcm-rng.c| 4 +---
>  drivers/char/hw_random/omap-rng.c| 4 +---
>  drivers/char/hw_random/pasemi-rng.c  | 4 +---
>  drivers/char/hw_random/pic32-rng.c   | 4 +---
>  drivers/char/hw_random/st-rng.c  | 4 +---
>  drivers/char/hw_random/tx4939-rng.c  | 4 +---
>  drivers/char/hw_random/xgene-rng.c   | 4 +---
>  13 files changed, 13 insertions(+), 40 deletions(-)
> 


-- 
Florian


Re: [PATCH -next 02/13] hwrng: bcm2835 - use devm_platform_ioremap_resource() to simplify code

2019-10-16 Thread Florian Fainelli
On 10/16/19 3:46 AM, YueHaibing wrote:
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
> 
> Signed-off-by: YueHaibing 

Acked-by: Florian Fainelli 
-- 
Florian


Re: [PATCH 2/4] dt-bindings: doc: Reflect new NVMEM of_get_mac_address behaviour

2019-04-27 Thread Florian Fainelli



On 4/26/2019 4:06 PM, Petr Štetiar wrote:
> As of_get_mac_address now supports NVMEM under the hood, we should
> update the bindings documentation with the new nvmem-cell* properties.
> While at it, fix also other missing properties supported by
> of_get_mac_address.
> 
> Signed-off-by: Petr Štetiar 

While I appreciate your effort in making the bindings up to date and
consistent, this does really scale well and is an error prone exercise,
how about consolidating all MAC address related properties into the
ethernet.txt document like you just did and update all bindings to
indicate something along the lines of:

For all other standard Ethernet related properties, please refer to
ethernet.txt or something like that?
-- 
Florian


Re: [PATCH] serial: 8250: Default SERIAL_OF_PLATFORM to SERIAL_8250

2018-12-20 Thread Florian Fainelli
Le 12/20/18 à 9:38 AM, Guenter Roeck a écrit :
> On Thu, Dec 20, 2018 at 04:21:11PM +0100, Greg Kroah-Hartman wrote:
>> On Wed, Nov 14, 2018 at 05:11:25PM -0800, Guenter Roeck wrote:
>>> On Thu, Nov 01, 2018 at 11:26:06AM -0700, Florian Fainelli wrote:
>>>> It is way too easy to miss enabling SERIAL_OF_PLATFORM which would
>>>> result in the inability for the kernel to have a valid console device,
>>>> which can be seen with:
>>>>
>>>> Warning: unable to open an initial console.
>>>>
>>>> and then:
>>>>
>>>> Run /init as init process
>>>> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0100
>>>>
>>>> Since SERIAL_OF_PLATFORM already depends on SERIAL_8250 && OF there
>>>> really is no drawback to defaulting this config to the value of
>>>> SERIAL_8250.
>>>>
>>>> Signed-off-by: Florian Fainelli 
>>>> Signed-off-by: Greg Kroah-Hartman 
>>>
>>> This patch results in situations where CONFIG_SERIAL_OF_PLATFORM is now
>>> defined where it was not previously. Example mpc85xx_defconfig. This in
>>> turn results in boot failures for those configurations, with an error
>>> message of
>>>
>>> of_serial: probe of e0004500.serial failed with error -22
>>>
>>> which wasn't seen before.
>>>
>>> Not sure if replacing a potential problem with a real one is really an
>>> improvement.`
>>
>> What ever was the result of this long thread?  Should I revert
>> something?  Or was a patch proposed?
>>
> The problem still exists in next-20181220.

I submitted a tentative patch to fix the problem, discussed it with
Michael and he had an alternative patch to 8250_core.c that should work
equally well though I was worried more breakage could be created that
way. Since clearly we have not been able to make much progress, maybe a
reversion of the original patch is appropriate, yes it's now sent a as a
reply to this mail! 

> 
> Unfortunately this is now just one failure of many in -next. I see more
> than 90 boot failures (out of ~330) there, not counting the build failures.
> And that is on a good day.
> 
> Guenter
> 


-- 
Florian


[PATCH] Revert "serial: 8250: Default SERIAL_OF_PLATFORM to SERIAL_8250"

2018-12-20 Thread Florian Fainelli
This reverts commit 6d11023c345e369bcb9d5a68b271764e362c1f6e ("serial:
8250: Default SERIAL_OF_PLATFORM to SERIAL_8250") since that breaks at
least mpc8544ds (PowerPC) using arch/powerpc/kernel/legacy_serial.c.

See https://lkml.org/lkml/2018/12/5/1491 for discussion and analysis

Fixes: 6d11023c345e ("serial: 8250: Default SERIAL_OF_PLATFORM to SERIAL_8250")
Signed-off-by: Florian Fainelli 
---
 drivers/tty/serial/8250/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index d7737dca0e48..15c2c5463835 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -484,7 +484,6 @@ config SERIAL_8250_PXA
 config SERIAL_OF_PLATFORM
tristate "Devicetree based probing for 8250 ports"
depends on SERIAL_8250 && OF
-   default SERIAL_8250
help
  This option is used for all 8250 compatible serial ports that
  are probed through devicetree, including Open Firmware based
-- 
2.19.1



Re: [PATCH] serial: 8250: Default SERIAL_OF_PLATFORM to SERIAL_8250

2018-12-05 Thread Florian Fainelli
On 12/4/18 9:47 PM, Michael Ellerman wrote:
> Hi Florian,
> 
> Florian Fainelli  writes:
>> +PPC folks
>>
>> On 11/23/18 10:20 AM, Guenter Roeck wrote:
>>> On Mon, Nov 19, 2018 at 12:50:50PM -0800, Guenter Roeck wrote:
>>>> On Mon, Nov 19, 2018 at 10:44:30AM -0800, Florian Fainelli wrote:
>>>>> On 11/15/18 5:16 PM, Guenter Roeck wrote:
>>>>>> On Thu, Nov 15, 2018 at 11:48:20AM -0800, Florian Fainelli wrote:
>>>>>>>
>>>>>>> OK, would you mind testing this below? It seems to me that 8250_of.c is
>>>>>>> incompatible with arch/powerpc/kernel/legacy_serial.c and that is what
>>>>>>> is causing the issue here.
>>>>>>>
>>>>>>> diff --git a/drivers/tty/serial/8250/Kconfig
>>>>>>> b/drivers/tty/serial/8250/Kconfig
>>>>>>> index d7737dca0e48..21cb14cbd34a 100644
>>>>>>> --- a/drivers/tty/serial/8250/Kconfig
>>>>>>> +++ b/drivers/tty/serial/8250/Kconfig
>>>>>>> @@ -483,7 +483,7 @@ config SERIAL_8250_PXA
>>>>>>>
>>>>>>>  config SERIAL_OF_PLATFORM
>>>>>>> tristate "Devicetree based probing for 8250 ports"
>>>>>>> -   depends on SERIAL_8250 && OF
>>>>>>> +   depends on SERIAL_8250 && OF && !(PPC && PPC_UDBG_16550)
>>>>>>> default SERIAL_8250
>>>>>>> help
>>>>>>>   This option is used for all 8250 compatible serial ports that
>>>>>>
>>>>>> 44x/virtex5_defconfig has both PPC_UDBG_16550 and SERIAL_OF_PLATFORM 
>>>>>> enabled
>>>>>> and fails to boot (or display anything on the console) with this patch 
>>>>>> applied.
>>>>>
>>>>> Thanks for trying, can you either share or provide a link to the mpc85xx
>>>>> and ml507 qemu command lines that you use? I spent a good chunk of my
>>>>> time trying to get a kernel to boot but has failed so far.
>>>>>
>>>>
>>>
>>> Any update ? I still see the boot failures in next-20181123.
>>
>> Yes, took most of last week's off sorry for the delay. I have finally
>> been able to boot a kernel using your instructions, thanks. The problem
>> is the following call chain:
>>
>> of_platform_serial_probe()
>>  -> serial8250_register_8250_port()
>> -> up->port.uartclk == 0, return -EINVAL
>>  -> irq_dispose_mapping()
>>
>> and then we basically unwind what we just did with
>> of_platform_serial_probe() including disabling the UART's IRQ, comment
>> out the irq_dispose_mapping() and you will have a functional console
>> again. 8250_of.c is clearly not a full replacement for legacy_serial.c
>> (that was a first attempt), so we need to keep that code around. Making
>> the depends even more conditional also does not sound too appealing,
>> because while we have identified mpc85xx as being problematic, who knows
>> about other platforms. So the best I have that does not involve a revert
>> is this below.
>>
>> Ben, Michael, would that sound reasonable to you? It seems to me that
>> there is a million ways to shoot the legacy_serial 8250 registration in
>> the foot in any cases, and having CONFIG_SERIAL_OF_PLATFORM just made it
>> painfully obvious.
> 
> We generally try to avoid changing the device tree from inside Linux,
> it's meant to describe the hardware as we're given it, not the state of
> Linux drivers etc. Perhaps this is an exceptional case but it still
> seems fishy.

Technically this is changing the Linux internal representation of a
Device Tree node reference (device_node), which is a bit away from
actual HW representation.

> 
> I also worry that marking the device node disabled might break something
> else, but that's maybe me being paranoid.

The code as it is today only makes use of device_node pointers/Device
Tree to find out about the UART base address and a bunch of properties,
but it registers the UART as platform_device without making use of the
existing OF infrastructure to do that (historical reasons really).

> 
> I guess I don't really understand how things are supposed to work
> though. We have the 8250 driver and also the OF platform driver, the
> former has already setup the device and then the OF driver just comes
> along and takes over?

When the OF platform driver comes along it tries to register the same
device instance, fails t

Re: [PATCH] serial: 8250: Default SERIAL_OF_PLATFORM to SERIAL_8250

2018-11-26 Thread Florian Fainelli
+PPC folks

On 11/23/18 10:20 AM, Guenter Roeck wrote:
> On Mon, Nov 19, 2018 at 12:50:50PM -0800, Guenter Roeck wrote:
>> On Mon, Nov 19, 2018 at 10:44:30AM -0800, Florian Fainelli wrote:
>>> On 11/15/18 5:16 PM, Guenter Roeck wrote:
>>>> On Thu, Nov 15, 2018 at 11:48:20AM -0800, Florian Fainelli wrote:
>>>>>
>>>>> OK, would you mind testing this below? It seems to me that 8250_of.c is
>>>>> incompatible with arch/powerpc/kernel/legacy_serial.c and that is what
>>>>> is causing the issue here.
>>>>>
>>>>> diff --git a/drivers/tty/serial/8250/Kconfig
>>>>> b/drivers/tty/serial/8250/Kconfig
>>>>> index d7737dca0e48..21cb14cbd34a 100644
>>>>> --- a/drivers/tty/serial/8250/Kconfig
>>>>> +++ b/drivers/tty/serial/8250/Kconfig
>>>>> @@ -483,7 +483,7 @@ config SERIAL_8250_PXA
>>>>>
>>>>>  config SERIAL_OF_PLATFORM
>>>>> tristate "Devicetree based probing for 8250 ports"
>>>>> -   depends on SERIAL_8250 && OF
>>>>> +   depends on SERIAL_8250 && OF && !(PPC && PPC_UDBG_16550)
>>>>> default SERIAL_8250
>>>>> help
>>>>>   This option is used for all 8250 compatible serial ports that
>>>>
>>>> 44x/virtex5_defconfig has both PPC_UDBG_16550 and SERIAL_OF_PLATFORM 
>>>> enabled
>>>> and fails to boot (or display anything on the console) with this patch 
>>>> applied.
>>>
>>> Thanks for trying, can you either share or provide a link to the mpc85xx
>>> and ml507 qemu command lines that you use? I spent a good chunk of my
>>> time trying to get a kernel to boot but has failed so far.
>>>
>>
> 
> Any update ? I still see the boot failures in next-20181123.

Yes, took most of last week's off sorry for the delay. I have finally
been able to boot a kernel using your instructions, thanks. The problem
is the following call chain:

of_platform_serial_probe()
 -> serial8250_register_8250_port()
-> up->port.uartclk == 0, return -EINVAL
-> irq_dispose_mapping()

and then we basically unwind what we just did with
of_platform_serial_probe() including disabling the UART's IRQ, comment
out the irq_dispose_mapping() and you will have a functional console
again. 8250_of.c is clearly not a full replacement for legacy_serial.c
(that was a first attempt), so we need to keep that code around. Making
the depends even more conditional also does not sound too appealing,
because while we have identified mpc85xx as being problematic, who knows
about other platforms. So the best I have that does not involve a revert
is this below.

Ben, Michael, would that sound reasonable to you? It seems to me that
there is a million ways to shoot the legacy_serial 8250 registration in
the foot in any cases, and having CONFIG_SERIAL_OF_PLATFORM just made it
painfully obvious.

diff --git a/arch/powerpc/kernel/legacy_serial.c
b/arch/powerpc/kernel/legacy_serial.c
index 33b34a58fc62..31353a27d714 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -16,7 +16,7 @@
 #include 
 #include 

-#undef DEBUG
+#define DEBUG

 #ifdef DEBUG
 #define DBG(fmt...) do { printk(fmt); } while(0)
@@ -70,6 +70,29 @@ static void tsi_serial_out(struct uart_port *p, int
offset, int value)
writeb(value, p->membase + offset);
 }

+#ifdef CONFIG_SERIAL_OF_PLATFORM
+static struct property uart_prop = {
+   .value = "disabled",
+   .name = "status",
+   .length = strlen("disabled"),
+};
+
+static void __init disable_uart_node(struct device_node *np)
+{
+   /* To avoid having 8250_of.c attempt to register the same device
+* and failing to do, and calling irq_dispose_mapping(), just
+* disable the device_node for now.
+*/
+   of_update_property(np, _prop);
+   pr_info("%s: disabled UART node\n", __func__);
+}
+#else
+static inline int disable_uart_node(struct device_node *np)
+{
+   return 0;
+}
+#endif
+
 static int __init add_legacy_port(struct device_node *np, int want_index,
  int iotype, phys_addr_t base,
  phys_addr_t taddr, unsigned long irq,
@@ -80,6 +103,8 @@ static int __init add_legacy_port(struct device_node
*np, int want_index,
u32 shift = 0;
int index;

+   disable_uart_node(np);
+
/* get clock freq. if present */
clk = of_get_property(np, "clock-frequency", NULL);
if (clk && *clk)



-- 
Florian


Re: ethernet "bus" number in DTS ?

2018-10-26 Thread Florian Fainelli
On 10/23/18 11:22 PM, Michal Suchánek wrote:
> On Tue, 23 Oct 2018 11:20:36 -0700
> Florian Fainelli  wrote:
> 
>> On 10/23/18 11:02 AM, Joakim Tjernlund wrote:
>>> On Tue, 2018-10-23 at 10:03 -0700, Florian Fainelli wrote:  
> 
>>>
>>> I also noted that using status = "disabled" didn't work either to
>>> create a fix name scheme. Even worse, all the eth I/F after gets
>>> renumbered. It seems to me there is value in having stability in
>>> eth I/F naming at boot. Then userspace(udev) can rename if need be. 
>>>
>>> Sure would like to known more about why this feature is not wanted ?
>>>
>>> I found
>>>   https://patchwork.kernel.org/patch/4122441/
>>> You quote policy as reason but surely it must be better to
>>> have something stable, connected to the hardware name, than
>>> semirandom naming?  
>>
>> If the Device Tree nodes are ordered by ascending base register
>> address, my understanding is that you get the same order as far as
>> platform_device creation goes, this may not be true in the future if
>> Rob decides to randomize that, but AFAICT this is still true. This
>> may not work well with status = disabled properties being inserted
>> here and there, but we have used that here and it has worked for as
>> far as I can remember doing it.
> 
> So this is unstable in several respects. First is changing the
> enabled/disabled status in the deivecetrees provided by the kernel.
> 
> Second is if you have hardware hotplug mechanism either by firmware or
> by loading device overlays.
> 
> Third is the case when the devicetree is not built as part of the
> kernel but is instead provided by firmware that initializes the
> low-level hardware details. Then the ordering by address is not
> guaranteed nor is that the same address will be used to access the same
> interface every time. There might be multiple ways to configure the
> hardware depending on firmware configuration and/or version.
> 
>  
>> Second, you might want to name network devices ethX, but what if I
>> want to name them ethernetX or fooX or barX? Should we be accepting a
>> mechanism in the kernel that would allow someone to name the
>> interfaces the way they want straight from a name being provided in
>> Device Tree?
> 
> Clearly if there is text Ethernet1 printed above the Ethernet port we
> should provide a mechanism to name the port Ethernet1 by default.

Yes, but then have a specific property that is flexible enough to
retrieve things like "vital product information". For DSA switches, we
have an optional "label" property which names the network device
directly like it would be found on the product's case. Ideally this
should be much more flexible such that it can point to the appropriate
node/firmware service/whatever to get that name, which is what some
people have been working on lately, see [1].

[1]: https://lkml.org/lkml/2018/8/14/1039

The point is don't re-purpose the aliases which is something that exists
within Device Tree to basically provide a shorter path to a specific set
of nodes for the boot program to mangle and muck with instead of having
to resolve a full path to a node. At least that is how I conceive it.

Now what complicates the matter is that some OSes like Linux tend to use
it to also generate seemingly stable index for peripherals that are
numbered by index such as SPI, I2C, etc buses, which is why we are
having this conversation here, see below for more.

> 
>>
>> Aliases are fine for providing relative stability within the Device
>> Tree itself and boot programs that might need to modify the Device
>> Tree (e.g: inserting MAC addresses) such that you don't have to
>> encode logic to search for nodes by compatible strings etc. but
>> outside of that use case, it seems to me that you can resolve every
>> naming decision in user-space.
> 
> However, this is pushing platform-specific knowledge to userspace. The
> way the Ethernet interface is attached and hence the device properties
> usable for identifying the device uniquely are platform-specific.

There is always going to be some amount of platform specific knowledge
to user-space, what matters is the level of abstraction that is
presented to you.

> 
> On the other hand, aliases are universal when provided. If they are
> good enough to assign a MAC address they are good enough to provide a
> stable default name.

We are not talking about the same aliases then. The special Device Tree
node named "aliases" is a way to create shorted Device Tree node paths,
it is not by any means an equivalent for what I would rather call a
"label", or "port name" or silk s

Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD

2018-10-26 Thread Florian Fainelli
On 10/26/18 4:07 AM, Mike Rapoport wrote:
> On Thu, Oct 25, 2018 at 04:07:13PM -0700, Florian Fainelli wrote:
>> On 10/25/18 2:13 PM, Rob Herring wrote:
>>> On Thu, Oct 25, 2018 at 12:30 PM Mike Rapoport  wrote:
>>>>
>>>> On Thu, Oct 25, 2018 at 08:15:15AM -0500, Rob Herring wrote:
>>>>> +Ard
>>>>>
>>>>> On Thu, Oct 25, 2018 at 4:38 AM Mike Rapoport  wrote:
>>>>>>
>>>>>> On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote:
>>>>>>> On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli  
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> While investigating why ARM64 required a ton of objects to be rebuilt
>>>>>>>> when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
>>>>>>>> because we define __early_init_dt_declare_initrd() differently and we 
>>>>>>>> do
>>>>>>>> that in arch/arm64/include/asm/memory.h which gets included by a fair
>>>>>>>> amount of other header files, and translation units as well.
>>>>>>>
>>>>>> I think arm64 does not have to redefine __early_init_dt_declare_initrd().
>>>>>> Something like this might be just all we need (completely untested,
>>>>>> probably it won't even compile):
> 
> [ ... ]
>  
>> FWIW, I am extracting the ARM implementation that parses the initrd
>> early command line parameter and the "setup" code doing the page
>> boundary alignment and memblock checking into a helper into lib/ that
>> other architectures can re-use. So far, this removes the need for
>> unicore32, arc and arm to duplicate essentially the same logic.
> 
> Presuming you are going to need asm-generic/initrd.h for that as well,
> using override for __early_init_dt_declare_initrd in arm64 version of
> initrd.h might be the simplest option.

What I am contemplating doing is promote
phys_initrd_start/phys_initrd_size to be global variables (similar to
initrd_start, initrd_end) and have a generic helper function for parsing
the initrd= command line parameter and finally removing
__early_init_dt_declare_initrd() because we could have
early_init_dt_check_for_initrd() just populate
phys_initrd_start/phys_initrd_size directly as well as
initrd_start/initrd_end using __va() to preserve compatibility with
architectures that rely on this. Then I would convert ARM64 to check for
phys_initrd_start which is really what it is is trying to do in
arch/arm64/mm/init.c::arm64_memblock_init().

Does that sound like a reasonable approach?
-- 
Florian


Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD

2018-10-25 Thread Florian Fainelli
On 10/25/18 2:13 PM, Rob Herring wrote:
> On Thu, Oct 25, 2018 at 12:30 PM Mike Rapoport  wrote:
>>
>> On Thu, Oct 25, 2018 at 08:15:15AM -0500, Rob Herring wrote:
>>> +Ard
>>>
>>> On Thu, Oct 25, 2018 at 4:38 AM Mike Rapoport  wrote:
>>>>
>>>> On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote:
>>>>> On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli  
>>>>> wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> While investigating why ARM64 required a ton of objects to be rebuilt
>>>>>> when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
>>>>>> because we define __early_init_dt_declare_initrd() differently and we do
>>>>>> that in arch/arm64/include/asm/memory.h which gets included by a fair
>>>>>> amount of other header files, and translation units as well.
>>>>>
>>>>> I scratch my head sometimes as to why some config options rebuild so
>>>>> much stuff. One down, ? to go. :)
>>>>>
>>>>>> Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build
>>>>>> systems that generate two kernels: one with the initramfs and one
>>>>>> without. buildroot is one of these build systems, OpenWrt is also
>>>>>> another one that does this.
>>>>>>
>>>>>> This patch series proposes adding an empty initrd.h to satisfy the need
>>>>>> for drivers/of/fdt.c to unconditionally include that file, and moves the
>>>>>> custom __early_init_dt_declare_initrd() definition away from
>>>>>> asm/memory.h
>>>>>>
>>>>>> This cuts the number of objects rebuilds from 1920 down to 26, so a
>>>>>> factor 73 approximately.
>>>>>>
>>>>>> Apologies for the long CC list, please let me know how you would go
>>>>>> about merging that and if another approach would be preferable, e.g:
>>>>>> introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
>>>>>> something like that.
>>>>>
>>>>> There may be a better way as of 4.20 because bootmem is now gone and
>>>>> only memblock is used. This should unify what each arch needs to do
>>>>> with initrd early. We need the physical address early for memblock
>>>>> reserving. Then later on we need the virtual address to access the
>>>>> initrd. Perhaps we should just change initrd_start and initrd_end to
>>>>> physical addresses (or add 2 new variables would be less invasive and
>>>>> allow for different translation than __va()). The sanity checks and
>>>>> memblock reserve could also perhaps be moved to a common location.
>>>>>
>>>>> Alternatively, given arm64 is the only oddball, I'd be fine with an
>>>>> "if (IS_ENABLED(CONFIG_ARM64))" condition in the default
>>>>> __early_init_dt_declare_initrd as long as we have a path to removing
>>>>> it like the above option.
>>>>
>>>> I think arm64 does not have to redefine __early_init_dt_declare_initrd().
>>>> Something like this might be just all we need (completely untested,
>>>> probably it won't even compile):
>>>>
>>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>>>> index 9d9582c..e9ca238 100644
>>>> --- a/arch/arm64/mm/init.c
>>>> +++ b/arch/arm64/mm/init.c
>>>> @@ -62,6 +62,9 @@ s64 memstart_addr __ro_after_init = -1;
>>>>  phys_addr_t arm64_dma_phys_limit __ro_after_init;
>>>>
>>>>  #ifdef CONFIG_BLK_DEV_INITRD
>>>> +
>>>> +static phys_addr_t initrd_start_phys, initrd_end_phys;
>>>> +
>>>>  static int __init early_initrd(char *p)
>>>>  {
>>>> unsigned long start, size;
>>>> @@ -71,8 +74,8 @@ static int __init early_initrd(char *p)
>>>> if (*endp == ',') {
>>>> size = memparse(endp + 1, NULL);
>>>>
>>>> -   initrd_start = start;
>>>> -   initrd_end = start + size;
>>>> +   initrd_start_phys = start;
>>>> +   initrd_end_phys = end;
>>>> }
>>>> return 0;
>>>>  }
>>>> @@ -407,14 +410,27 @@ void __init arm64_memblock_init(void)
>>>

Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD

2018-10-24 Thread Florian Fainelli
On 10/24/18 12:55 PM, Rob Herring wrote:
> On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli  wrote:
>>
>> Hi all,
>>
>> While investigating why ARM64 required a ton of objects to be rebuilt
>> when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
>> because we define __early_init_dt_declare_initrd() differently and we do
>> that in arch/arm64/include/asm/memory.h which gets included by a fair
>> amount of other header files, and translation units as well.
> 
> I scratch my head sometimes as to why some config options rebuild so
> much stuff. One down, ? to go. :)
> 

This one was by far the most invasive one due to its include chain, but
yes, there would be many more that could be optimized.

>> Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build
>> systems that generate two kernels: one with the initramfs and one
>> without. buildroot is one of these build systems, OpenWrt is also
>> another one that does this.
>>
>> This patch series proposes adding an empty initrd.h to satisfy the need
>> for drivers/of/fdt.c to unconditionally include that file, and moves the
>> custom __early_init_dt_declare_initrd() definition away from
>> asm/memory.h
>>
>> This cuts the number of objects rebuilds from 1920 down to 26, so a
>> factor 73 approximately.
>>
>> Apologies for the long CC list, please let me know how you would go
>> about merging that and if another approach would be preferable, e.g:
>> introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
>> something like that.
> 
> There may be a better way as of 4.20 because bootmem is now gone and
> only memblock is used. This should unify what each arch needs to do
> with initrd early. We need the physical address early for memblock
> reserving. Then later on we need the virtual address to access the
> initrd. Perhaps we should just change initrd_start and initrd_end to
> physical addresses (or add 2 new variables would be less invasive and
> allow for different translation than __va()). The sanity checks and
> memblock reserve could also perhaps be moved to a common location.
> 
> Alternatively, given arm64 is the only oddball, I'd be fine with an
> "if (IS_ENABLED(CONFIG_ARM64))" condition in the default
> __early_init_dt_declare_initrd as long as we have a path to removing
> it like the above option.

OK, let me cook a patch doing that and meanwhile I will look at how much
work is involved to implement the above option you outlined, which also
sounds entirely reasonable.

Thanks!
-- 
Florian


[PATCH v2 2/2] arm64: Create asm/initrd.h

2018-10-24 Thread Florian Fainelli
ARM64 is the only architecture that requires a re-definition of
__early_init_dt_declare_initrd(). Now that we added the infrastructure
in asm-generic to provide an asm/initrd.h file, properly break up that
definition from asm/memory.h and make use of that header in
drivers/of/fdt.c where this is used.

This significantly cuts the number of objects that need to be rebuilt on
ARM64 due to the repercusions of including asm/memory.h in several
places.

Signed-off-by: Florian Fainelli 
---
 arch/arm64/include/asm/initrd.h | 13 +
 arch/arm64/include/asm/memory.h |  8 
 drivers/of/fdt.c|  1 +
 3 files changed, 14 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/include/asm/initrd.h

diff --git a/arch/arm64/include/asm/initrd.h b/arch/arm64/include/asm/initrd.h
new file mode 100644
index ..0c9572485810
--- /dev/null
+++ b/arch/arm64/include/asm/initrd.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_INITRD_H
+#define __ASM_INITRD_H
+
+#ifdef CONFIG_BLK_DEV_INITRD
+#define __early_init_dt_declare_initrd(__start, __end) \
+   do {\
+   initrd_start = (__start);   \
+   initrd_end = (__end);   \
+   } while (0)
+#endif
+
+#endif /* __ASM_INITRD_H */
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..dc3ca21ba240 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -168,14 +168,6 @@
 #define IOREMAP_MAX_ORDER  (PMD_SHIFT)
 #endif
 
-#ifdef CONFIG_BLK_DEV_INITRD
-#define __early_init_dt_declare_initrd(__start, __end) \
-   do {\
-   initrd_start = (__start);   \
-   initrd_end = (__end);   \
-   } while (0)
-#endif
-
 #ifndef __ASSEMBLY__
 
 #include 
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252cf9c..4e4711af907b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -28,6 +28,7 @@
 
 #include   /* for COMMAND_LINE_SIZE */
 #include 
+#include 
 
 #include "of_private.h"
 
-- 
2.17.1



[PATCH v2 1/2] arch: Add asm-generic/initrd.h and make use of it for most architectures

2018-10-24 Thread Florian Fainelli
In preparation for separating the definition of
__early_init_dt_declare_initrd() on ARM64 in order to cut the amount of
files that require a rebuild when CONFIG_BLK_DEV_INITRD value is
changed, introduce an empty asm-generic initrd.h file and update all
architectures but arm64 to make use of it.

Signed-off-by: Florian Fainelli 
---
 arch/alpha/include/asm/Kbuild  | 1 +
 arch/arc/include/asm/Kbuild| 1 +
 arch/arm/include/asm/Kbuild| 1 +
 arch/c6x/include/asm/Kbuild| 1 +
 arch/h8300/include/asm/Kbuild  | 1 +
 arch/hexagon/include/asm/Kbuild| 1 +
 arch/ia64/include/asm/Kbuild   | 1 +
 arch/m68k/include/asm/Kbuild   | 1 +
 arch/microblaze/include/asm/Kbuild | 1 +
 arch/mips/include/asm/Kbuild   | 1 +
 arch/nds32/include/asm/Kbuild  | 1 +
 arch/nios2/include/asm/Kbuild  | 1 +
 arch/openrisc/include/asm/Kbuild   | 1 +
 arch/parisc/include/asm/Kbuild | 1 +
 arch/powerpc/include/asm/Kbuild| 1 +
 arch/riscv/include/asm/Kbuild  | 1 +
 arch/s390/include/asm/Kbuild   | 1 +
 arch/sh/include/asm/Kbuild | 1 +
 arch/sparc/include/asm/Kbuild  | 1 +
 arch/um/include/asm/Kbuild | 1 +
 arch/unicore32/include/asm/Kbuild  | 1 +
 arch/x86/include/asm/Kbuild| 1 +
 arch/xtensa/include/asm/Kbuild | 1 +
 include/asm-generic/initrd.h   | 1 +
 24 files changed, 24 insertions(+)
 create mode 100644 include/asm-generic/initrd.h

diff --git a/arch/alpha/include/asm/Kbuild b/arch/alpha/include/asm/Kbuild
index 0580cb8c84b2..cd6f723aed1b 100644
--- a/arch/alpha/include/asm/Kbuild
+++ b/arch/alpha/include/asm/Kbuild
@@ -5,6 +5,7 @@ generic-y += compat.h
 generic-y += exec.h
 generic-y += export.h
 generic-y += fb.h
+generic-y += initrd.h
 generic-y += irq_work.h
 generic-y += mcs_spinlock.h
 generic-y += mm-arch-hooks.h
diff --git a/arch/arc/include/asm/Kbuild b/arch/arc/include/asm/Kbuild
index feed50ce89fa..ba18632aa493 100644
--- a/arch/arc/include/asm/Kbuild
+++ b/arch/arc/include/asm/Kbuild
@@ -10,6 +10,7 @@ generic-y += fb.h
 generic-y += ftrace.h
 generic-y += hardirq.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kmap_types.h
diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 1d66db9c9db5..b91d5b32e64f 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -4,6 +4,7 @@ generic-y += early_ioremap.h
 generic-y += emergency-restart.h
 generic-y += exec.h
 generic-y += extable.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += kdebug.h
 generic-y += local.h
diff --git a/arch/c6x/include/asm/Kbuild b/arch/c6x/include/asm/Kbuild
index 33a2c94fed0d..9e14cf6e89b4 100644
--- a/arch/c6x/include/asm/Kbuild
+++ b/arch/c6x/include/asm/Kbuild
@@ -13,6 +13,7 @@ generic-y += extable.h
 generic-y += fb.h
 generic-y += futex.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += io.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
diff --git a/arch/h8300/include/asm/Kbuild b/arch/h8300/include/asm/Kbuild
index a5d0b2991f47..7d4e06a757c8 100644
--- a/arch/h8300/include/asm/Kbuild
+++ b/arch/h8300/include/asm/Kbuild
@@ -19,6 +19,7 @@ generic-y += futex.h
 generic-y += hardirq.h
 generic-y += hash.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kdebug.h
diff --git a/arch/hexagon/include/asm/Kbuild b/arch/hexagon/include/asm/Kbuild
index 47c4da3d64a4..0be62abf2123 100644
--- a/arch/hexagon/include/asm/Kbuild
+++ b/arch/hexagon/include/asm/Kbuild
@@ -13,6 +13,7 @@ generic-y += fb.h
 generic-y += ftrace.h
 generic-y += hardirq.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += iomap.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index 557bbc8ba9f5..1a1f1e4ba0d5 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -1,5 +1,6 @@
 generic-y += compat.h
 generic-y += exec.h
+generic-y += initrd.h
 generic-y += irq_work.h
 generic-y += mcs_spinlock.h
 generic-y += mm-arch-hooks.h
diff --git a/arch/m68k/include/asm/Kbuild b/arch/m68k/include/asm/Kbuild
index a4b8d3331a9e..9903551e0c9c 100644
--- a/arch/m68k/include/asm/Kbuild
+++ b/arch/m68k/include/asm/Kbuild
@@ -7,6 +7,7 @@ generic-y += exec.h
 generic-y += extable.h
 generic-y += futex.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kdebug.h
diff --git a/arch/microblaze/include/asm/Kbuild 
b/arch/microblaze/include/asm/Kbuild
index 569ba9e670c1..ec37e6304be5 100644
--- a/arch/microblaze/include/asm/Kbuild
+++ b/arch/microblaze/include/asm/Kbuild
@@ -11,6 +11,7 @@ generic-y += exec.h
 generic-y += extable.h
 generic-y += fb.h
 generic-y += hardirq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kdebug.h
diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
index

[PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD

2018-10-24 Thread Florian Fainelli
Hi all,

While investigating why ARM64 required a ton of objects to be rebuilt
when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
because we define __early_init_dt_declare_initrd() differently and we do
that in arch/arm64/include/asm/memory.h which gets included by a fair
amount of other header files, and translation units as well.

Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build
systems that generate two kernels: one with the initramfs and one
without. buildroot is one of these build systems, OpenWrt is also
another one that does this.

This patch series proposes adding an empty initrd.h to satisfy the need
for drivers/of/fdt.c to unconditionally include that file, and moves the
custom __early_init_dt_declare_initrd() definition away from
asm/memory.h

This cuts the number of objects rebuilds from 1920 down to 26, so a
factor 73 approximately.

Apologies for the long CC list, please let me know how you would go
about merging that and if another approach would be preferable, e.g:
introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
something like that.

Changes in v2:

- put an /* empty */ comment in the asm-generic/initrd.h file
- trim down the CC list to maximize the chances of people receiving this

Florian Fainelli (2):
  arch: Add asm-generic/initrd.h and make use of it for most
architectures
  arm64: Create asm/initrd.h

 arch/alpha/include/asm/Kbuild  |  1 +
 arch/arc/include/asm/Kbuild|  1 +
 arch/arm/include/asm/Kbuild|  1 +
 arch/arm64/include/asm/initrd.h| 13 +
 arch/arm64/include/asm/memory.h|  8 
 arch/c6x/include/asm/Kbuild|  1 +
 arch/h8300/include/asm/Kbuild  |  1 +
 arch/hexagon/include/asm/Kbuild|  1 +
 arch/ia64/include/asm/Kbuild   |  1 +
 arch/m68k/include/asm/Kbuild   |  1 +
 arch/microblaze/include/asm/Kbuild |  1 +
 arch/mips/include/asm/Kbuild   |  1 +
 arch/nds32/include/asm/Kbuild  |  1 +
 arch/nios2/include/asm/Kbuild  |  1 +
 arch/openrisc/include/asm/Kbuild   |  1 +
 arch/parisc/include/asm/Kbuild |  1 +
 arch/powerpc/include/asm/Kbuild|  1 +
 arch/riscv/include/asm/Kbuild  |  1 +
 arch/s390/include/asm/Kbuild   |  1 +
 arch/sh/include/asm/Kbuild |  1 +
 arch/sparc/include/asm/Kbuild  |  1 +
 arch/um/include/asm/Kbuild |  1 +
 arch/unicore32/include/asm/Kbuild  |  1 +
 arch/x86/include/asm/Kbuild|  1 +
 arch/xtensa/include/asm/Kbuild |  1 +
 drivers/of/fdt.c   |  1 +
 include/asm-generic/initrd.h   |  1 +
 27 files changed, 38 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/include/asm/initrd.h
 create mode 100644 include/asm-generic/initrd.h

-- 
2.17.1



[PATCH 2/2] arm64: Create asm/initrd.h

2018-10-24 Thread Florian Fainelli
ARM64 is the only architecture that requires a re-definition of
__early_init_dt_declare_initrd(). Now that we added the infrastructure
in asm-generic to provide an asm/initrd.h file, properly break up that
definition from asm/memory.h and make use of that header in
drivers/of/fdt.c where this is used.

This significantly cuts the number of objects that need to be rebuilt on
ARM64 due to the repercusions of including asm/memory.h in several
places.

Signed-off-by: Florian Fainelli 
---
 arch/arm64/include/asm/initrd.h | 13 +
 arch/arm64/include/asm/memory.h |  8 
 drivers/of/fdt.c|  1 +
 3 files changed, 14 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/include/asm/initrd.h

diff --git a/arch/arm64/include/asm/initrd.h b/arch/arm64/include/asm/initrd.h
new file mode 100644
index ..0c9572485810
--- /dev/null
+++ b/arch/arm64/include/asm/initrd.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_INITRD_H
+#define __ASM_INITRD_H
+
+#ifdef CONFIG_BLK_DEV_INITRD
+#define __early_init_dt_declare_initrd(__start, __end) \
+   do {\
+   initrd_start = (__start);   \
+   initrd_end = (__end);   \
+   } while (0)
+#endif
+
+#endif /* __ASM_INITRD_H */
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..dc3ca21ba240 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -168,14 +168,6 @@
 #define IOREMAP_MAX_ORDER  (PMD_SHIFT)
 #endif
 
-#ifdef CONFIG_BLK_DEV_INITRD
-#define __early_init_dt_declare_initrd(__start, __end) \
-   do {\
-   initrd_start = (__start);   \
-   initrd_end = (__end);   \
-   } while (0)
-#endif
-
 #ifndef __ASSEMBLY__
 
 #include 
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252cf9c..4e4711af907b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -28,6 +28,7 @@
 
 #include   /* for COMMAND_LINE_SIZE */
 #include 
+#include 
 
 #include "of_private.h"
 
-- 
2.17.1



[PATCH 1/2] arch: Add asm-generic/initrd.h and make use of it for most architectures

2018-10-24 Thread Florian Fainelli
In preparation for separating the definition of
__early_init_dt_declare_initrd() on ARM64 in order to cut the amount of
files that require a rebuild when CONFIG_BLK_DEV_INITRD value is
changed, introduce an empty asm-generic initrd.h file and update all
architectures but arm64 to make use of it.

Signed-off-by: Florian Fainelli 
---
 arch/alpha/include/asm/Kbuild  | 1 +
 arch/arc/include/asm/Kbuild| 1 +
 arch/arm/include/asm/Kbuild| 1 +
 arch/c6x/include/asm/Kbuild| 1 +
 arch/h8300/include/asm/Kbuild  | 1 +
 arch/hexagon/include/asm/Kbuild| 1 +
 arch/ia64/include/asm/Kbuild   | 1 +
 arch/m68k/include/asm/Kbuild   | 1 +
 arch/microblaze/include/asm/Kbuild | 1 +
 arch/mips/include/asm/Kbuild   | 1 +
 arch/nds32/include/asm/Kbuild  | 1 +
 arch/nios2/include/asm/Kbuild  | 1 +
 arch/openrisc/include/asm/Kbuild   | 1 +
 arch/parisc/include/asm/Kbuild | 1 +
 arch/powerpc/include/asm/Kbuild| 1 +
 arch/riscv/include/asm/Kbuild  | 1 +
 arch/s390/include/asm/Kbuild   | 1 +
 arch/sh/include/asm/Kbuild | 1 +
 arch/sparc/include/asm/Kbuild  | 1 +
 arch/um/include/asm/Kbuild | 1 +
 arch/unicore32/include/asm/Kbuild  | 1 +
 arch/x86/include/asm/Kbuild| 1 +
 arch/xtensa/include/asm/Kbuild | 1 +
 include/asm-generic/initrd.h   | 1 +
 24 files changed, 24 insertions(+)
 create mode 100644 include/asm-generic/initrd.h

diff --git a/arch/alpha/include/asm/Kbuild b/arch/alpha/include/asm/Kbuild
index 0580cb8c84b2..cd6f723aed1b 100644
--- a/arch/alpha/include/asm/Kbuild
+++ b/arch/alpha/include/asm/Kbuild
@@ -5,6 +5,7 @@ generic-y += compat.h
 generic-y += exec.h
 generic-y += export.h
 generic-y += fb.h
+generic-y += initrd.h
 generic-y += irq_work.h
 generic-y += mcs_spinlock.h
 generic-y += mm-arch-hooks.h
diff --git a/arch/arc/include/asm/Kbuild b/arch/arc/include/asm/Kbuild
index feed50ce89fa..ba18632aa493 100644
--- a/arch/arc/include/asm/Kbuild
+++ b/arch/arc/include/asm/Kbuild
@@ -10,6 +10,7 @@ generic-y += fb.h
 generic-y += ftrace.h
 generic-y += hardirq.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kmap_types.h
diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 1d66db9c9db5..b91d5b32e64f 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -4,6 +4,7 @@ generic-y += early_ioremap.h
 generic-y += emergency-restart.h
 generic-y += exec.h
 generic-y += extable.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += kdebug.h
 generic-y += local.h
diff --git a/arch/c6x/include/asm/Kbuild b/arch/c6x/include/asm/Kbuild
index 33a2c94fed0d..9e14cf6e89b4 100644
--- a/arch/c6x/include/asm/Kbuild
+++ b/arch/c6x/include/asm/Kbuild
@@ -13,6 +13,7 @@ generic-y += extable.h
 generic-y += fb.h
 generic-y += futex.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += io.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
diff --git a/arch/h8300/include/asm/Kbuild b/arch/h8300/include/asm/Kbuild
index a5d0b2991f47..7d4e06a757c8 100644
--- a/arch/h8300/include/asm/Kbuild
+++ b/arch/h8300/include/asm/Kbuild
@@ -19,6 +19,7 @@ generic-y += futex.h
 generic-y += hardirq.h
 generic-y += hash.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kdebug.h
diff --git a/arch/hexagon/include/asm/Kbuild b/arch/hexagon/include/asm/Kbuild
index 47c4da3d64a4..0be62abf2123 100644
--- a/arch/hexagon/include/asm/Kbuild
+++ b/arch/hexagon/include/asm/Kbuild
@@ -13,6 +13,7 @@ generic-y += fb.h
 generic-y += ftrace.h
 generic-y += hardirq.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += iomap.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index 557bbc8ba9f5..1a1f1e4ba0d5 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -1,5 +1,6 @@
 generic-y += compat.h
 generic-y += exec.h
+generic-y += initrd.h
 generic-y += irq_work.h
 generic-y += mcs_spinlock.h
 generic-y += mm-arch-hooks.h
diff --git a/arch/m68k/include/asm/Kbuild b/arch/m68k/include/asm/Kbuild
index a4b8d3331a9e..9903551e0c9c 100644
--- a/arch/m68k/include/asm/Kbuild
+++ b/arch/m68k/include/asm/Kbuild
@@ -7,6 +7,7 @@ generic-y += exec.h
 generic-y += extable.h
 generic-y += futex.h
 generic-y += hw_irq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kdebug.h
diff --git a/arch/microblaze/include/asm/Kbuild 
b/arch/microblaze/include/asm/Kbuild
index 569ba9e670c1..ec37e6304be5 100644
--- a/arch/microblaze/include/asm/Kbuild
+++ b/arch/microblaze/include/asm/Kbuild
@@ -11,6 +11,7 @@ generic-y += exec.h
 generic-y += extable.h
 generic-y += fb.h
 generic-y += hardirq.h
+generic-y += initrd.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
 generic-y += kdebug.h
diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
index

[PATCH 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD

2018-10-24 Thread Florian Fainelli
Hi all,

While investigating why ARM64 required a ton of objects to be rebuilt
when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
because we define __early_init_dt_declare_initrd() differently and we do
that in arch/arm64/include/asm/memory.h which gets included by a fair
amount of other header files, and translation units as well.

This patch series proposes adding an empty initrd.h to satisfy the need
for drivers/of/fdt.c to unconditionally include that file, and moves the
custom __early_init_dt_declare_initrd() definition away from
asm/memory.h

This cuts the number of objects rebuilds from 1920 down to 26, so a
factor 73 approximately.

Apologies for the long CC list, please let me know how you would go
about merging that and if another approach would be preferable, e.g:
introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
something like that.

Florian Fainelli (2):
  arch: Add asm-generic/initrd.h and make use of it for most
architectures
  arm64: Create asm/initrd.h

 arch/alpha/include/asm/Kbuild  |  1 +
 arch/arc/include/asm/Kbuild|  1 +
 arch/arm/include/asm/Kbuild|  1 +
 arch/arm64/include/asm/initrd.h| 13 +
 arch/arm64/include/asm/memory.h|  8 
 arch/c6x/include/asm/Kbuild|  1 +
 arch/h8300/include/asm/Kbuild  |  1 +
 arch/hexagon/include/asm/Kbuild|  1 +
 arch/ia64/include/asm/Kbuild   |  1 +
 arch/m68k/include/asm/Kbuild   |  1 +
 arch/microblaze/include/asm/Kbuild |  1 +
 arch/mips/include/asm/Kbuild   |  1 +
 arch/nds32/include/asm/Kbuild  |  1 +
 arch/nios2/include/asm/Kbuild  |  1 +
 arch/openrisc/include/asm/Kbuild   |  1 +
 arch/parisc/include/asm/Kbuild |  1 +
 arch/powerpc/include/asm/Kbuild|  1 +
 arch/riscv/include/asm/Kbuild  |  1 +
 arch/s390/include/asm/Kbuild   |  1 +
 arch/sh/include/asm/Kbuild |  1 +
 arch/sparc/include/asm/Kbuild  |  1 +
 arch/um/include/asm/Kbuild |  1 +
 arch/unicore32/include/asm/Kbuild  |  1 +
 arch/x86/include/asm/Kbuild|  1 +
 arch/xtensa/include/asm/Kbuild |  1 +
 drivers/of/fdt.c   |  1 +
 include/asm-generic/initrd.h   |  1 +
 27 files changed, 38 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/include/asm/initrd.h
 create mode 100644 include/asm-generic/initrd.h

-- 
2.17.1



Re: ethernet "bus" number in DTS ?

2018-10-23 Thread Florian Fainelli
On 10/23/18 1:02 PM, Joakim Tjernlund wrote:
> On Tue, 2018-10-23 at 11:20 -0700, Florian Fainelli wrote:
>>
>> On 10/23/18 11:02 AM, Joakim Tjernlund wrote:
>>> On Tue, 2018-10-23 at 10:03 -0700, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 10/23/18 9:49 AM, Joakim Tjernlund wrote:
>>>>> SPI (and others) has a way to define bus number in a aliases:
>>>>>   aliases {
>>>>>   ethernet4 = 
>>>>>   ethernet0 = 
>>>>>   ethernet1 = 
>>>>>   ethernet2 = 
>>>>>   ethernet3 = 
>>>>>   spi0 = 
>>>>>   };
>>>>> The 0 in the spi0 alias will translate to bus num 0 so one can control 
>>>>> the /dev nodes, like /dev/spidev0
>>>>> I am looking for the same for ethernet devices:
>>>>>  ethernet4 =   /* should become eth4 */
>>>>>  ethernet0 =   /* should become eth0 */
>>>>> but I cannot find something like that for eth devices.
>>>>>
>>>>> Could such functionality be added?
>>>>
>>>> It could, do we want and need to, no. You have the Ethernet alias in
>>>> /sys/class/net/*/device/uevent already that would allow you to perform
>>>> that (re)naming in user-space:
>>>>
>>>> # cat /sys/class/net/eth0/device/uevent
>>>> DRIVER=bcmgenet
>>>> OF_NAME=ethernet
>>>> OF_FULLNAME=/rdb/ethernet@f048
>>>> OF_TYPE=network
>>>> OF_COMPATIBLE_0=brcm,genet-v5
>>>> OF_COMPATIBLE_N=1
>>>> OF_ALIAS_0=eth0 <==
>>>> MODALIAS=of:NethernetTnetworkCbrcm,genet-v5
>>>
>>> Yes, one can if one uses udev and can find something to identify the hw I/F 
>>> with, my
>>> cat /sys/class/net/eth0/device/uevent looks like:
>>> DRIVER=fsl_dpa
>>> MODALIAS=platform:dpaa-ethernet
>>
>> Does not dpaa have a notion of Ethernet ports and those should have
>> proper information? Maybe that is part of your problem here, it should
>> have the OF_ALIAS information somehow available.
> 
> I cannot say ATM, but this lack of standard does not make it easier to rename 
> I/F's in udev.
> 
>>
>>> not sure mdev supports this, does it?
>>> Our simple installer FS(initramfs) doesn't have either udev or mdev.
>>
>> I don't know, but you could have a simple shell script that looks at
>> specific network device properties to decide on the naming and call
>> ifrename.
> 
> This reinventing of the wheel is what I am trying to avoid.

Embedded is all about being a special snowflake and re-inventing the
wheel, but having some desktop-like distribution user-space would
certainly allow you to re-invent other parts of the wheel.

> 
>>
>>> I also noted that using status = "disabled" didn't work either to create a 
>>> fix name scheme.
>>> Even worse, all the eth I/F after gets renumbered. It seems to me there
>>> is value in having stability in eth I/F naming at boot.
>>> Then userspace(udev) can rename if need be.
>>>
>>> Sure would like to known more about why this feature is not wanted ?
>>>
>>> I found
>>>   https://patchwork.kernel.org/patch/4122441/
>>> You quote policy as reason but surely it must be better to
>>> have something stable, connected to the hardware name, than semirandom 
>>> naming?
>>
>> If the Device Tree nodes are ordered by ascending base register address,
>> my understanding is that you get the same order as far as
>> platform_device creation goes, this may not be true in the future if Rob
>> decides to randomize that, but AFAICT this is still true. This may not
>> work well with status = disabled properties being inserted here and
>> there, but we have used that here and it has worked for as far as I can
>> remember doing it.
> 
> I recall it is the order in which the eth alias appear that controls the 
> naming,
> not 100% sure though.

Aliases are not looked up at all by the platform bus code other that
with of_get_alias() and friends, it is the order in which the nodes are
declared in the Device Tree, preferably ordered by base address that
dictates the order in which platform devices are created.

> 
>>
>> Second, you might want to name network devices ethX, but what if I want
>> to name them ethernetX or fooX or barX? Should we be accepting a
>> mechanism in the kernel that would allow someone to name t

Re: ethernet "bus" number in DTS ?

2018-10-23 Thread Florian Fainelli
On 10/23/18 11:02 AM, Joakim Tjernlund wrote:
> On Tue, 2018-10-23 at 10:03 -0700, Florian Fainelli wrote:
>> CAUTION: This email originated from outside of the organization. Do not 
>> click links or open attachments unless you recognize the sender and know the 
>> content is safe.
>>
>>
>> On 10/23/18 9:49 AM, Joakim Tjernlund wrote:
>>> SPI (and others) has a way to define bus number in a aliases:
>>>   aliases {
>>>   ethernet4 = 
>>>   ethernet0 = 
>>>   ethernet1 = 
>>>   ethernet2 = 
>>>   ethernet3 = 
>>>   spi0 = 
>>>   };
>>> The 0 in the spi0 alias will translate to bus num 0 so one can control the 
>>> /dev nodes, like /dev/spidev0
>>> I am looking for the same for ethernet devices:
>>>  ethernet4 =   /* should become eth4 */
>>>  ethernet0 =   /* should become eth0 */
>>> but I cannot find something like that for eth devices.
>>>
>>> Could such functionality be added?
>>
>> It could, do we want and need to, no. You have the Ethernet alias in
>> /sys/class/net/*/device/uevent already that would allow you to perform
>> that (re)naming in user-space:
>>
>> # cat /sys/class/net/eth0/device/uevent
>> DRIVER=bcmgenet
>> OF_NAME=ethernet
>> OF_FULLNAME=/rdb/ethernet@f048
>> OF_TYPE=network
>> OF_COMPATIBLE_0=brcm,genet-v5
>> OF_COMPATIBLE_N=1
>> OF_ALIAS_0=eth0 <==
>> MODALIAS=of:NethernetTnetworkCbrcm,genet-v5
> 
> Yes, one can if one uses udev and can find something to identify the hw I/F 
> with, my
> cat /sys/class/net/eth0/device/uevent looks like:
> DRIVER=fsl_dpa
> MODALIAS=platform:dpaa-ethernet

Does not dpaa have a notion of Ethernet ports and those should have
proper information? Maybe that is part of your problem here, it should
have the OF_ALIAS information somehow available.

> 
> not sure mdev supports this, does it?
> Our simple installer FS(initramfs) doesn't have either udev or mdev.

I don't know, but you could have a simple shell script that looks at
specific network device properties to decide on the naming and call
ifrename.

> 
> I also noted that using status = "disabled" didn't work either to create a 
> fix name scheme.
> Even worse, all the eth I/F after gets renumbered. It seems to me there
> is value in having stability in eth I/F naming at boot.
> Then userspace(udev) can rename if need be. 
> 
> Sure would like to known more about why this feature is not wanted ?
> 
> I found
>   https://patchwork.kernel.org/patch/4122441/
> You quote policy as reason but surely it must be better to
> have something stable, connected to the hardware name, than semirandom naming?

If the Device Tree nodes are ordered by ascending base register address,
my understanding is that you get the same order as far as
platform_device creation goes, this may not be true in the future if Rob
decides to randomize that, but AFAICT this is still true. This may not
work well with status = disabled properties being inserted here and
there, but we have used that here and it has worked for as far as I can
remember doing it.

Second, you might want to name network devices ethX, but what if I want
to name them ethernetX or fooX or barX? Should we be accepting a
mechanism in the kernel that would allow someone to name the interfaces
the way they want straight from a name being provided in Device Tree?

Aliases are fine for providing relative stability within the Device Tree
itself and boot programs that might need to modify the Device Tree (e.g:
inserting MAC addresses) such that you don't have to encode logic to
search for nodes by compatible strings etc. but outside of that use
case, it seems to me that you can resolve every naming decision in
user-space.
-- 
Florian


Re: ethernet "bus" number in DTS ?

2018-10-23 Thread Florian Fainelli
On 10/23/18 9:49 AM, Joakim Tjernlund wrote:
> SPI (and others) has a way to define bus number in a aliases:
>   aliases {
>   ethernet4 = 
>   ethernet0 = 
>   ethernet1 = 
>   ethernet2 = 
>   ethernet3 = 
>   spi0 = 
>   };
> The 0 in the spi0 alias will translate to bus num 0 so one can control the 
> /dev nodes, like /dev/spidev0
> I am looking for the same for ethernet devices:
>  ethernet4 =   /* should become eth4 */
>  ethernet0 =   /* should become eth0 */
> but I cannot find something like that for eth devices.
> 
> Could such functionality be added?

It could, do we want and need to, no. You have the Ethernet alias in
/sys/class/net/*/device/uevent already that would allow you to perform
that (re)naming in user-space:

# cat /sys/class/net/eth0/device/uevent
DRIVER=bcmgenet
OF_NAME=ethernet
OF_FULLNAME=/rdb/ethernet@f048
OF_TYPE=network
OF_COMPATIBLE_0=brcm,genet-v5
OF_COMPATIBLE_N=1
OF_ALIAS_0=eth0 <==
MODALIAS=of:NethernetTnetworkCbrcm,genet-v5
-- 
Florian


Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32

2018-09-24 Thread Florian Fainelli
On 09/24/2018 12:04 PM, Corentin Labbe wrote:
> This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.
> 
> Signed-off-by: Corentin Labbe 
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 56 
> +-
>  1 file changed, 22 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> index c5979569fd60..abcf65588576 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "stmmac_platform.h"
>  
> @@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
>   struct clk_gate rgmii_tx_en;
>  };
>  
> -static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
> - u32 mask, u32 value)
> -{
> - u32 data;
> -
> - data = readl(dwmac->regs + reg);
> - data &= ~mask;
> - data |= (value & mask);
> -
> - writel(data, dwmac->regs + reg);
> -}

Why not make mseon8b_dwmac_mask_bits() a wrapper around
clrsetbits_le32() whose purpose is only to dereference dwmac->regs and
pass it to clrsetbits_le32()? That would be far less changes to review
and audit for correctness, same goes with every other patch in this
series touching the meson drivers.
-- 
Florian


Re: [PATCH v4] watchdog: add SPDX identifiers for watchdog subsystem

2018-02-28 Thread Florian Fainelli
On 02/28/2018 07:01 AM, Marcus Folkesson wrote:
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
>   license
> 
> Signed-off-by: Marcus Folkesson <marcus.folkes...@gmail.com>
> Acked-by: Adam Thomson <adam.thomson.opensou...@diasemi.com>
> Acked-by: Baruch Siach <bar...@tkos.co.il>
> Acked-by: Charles Keepax <ckee...@opensource.cirrus.com>
> Acked-by: Keiji Hayashibara <hayashibara.ke...@socionext.com>
> Acked-by: Johannes Thumshirn <j...@kernel.org>
> Acked-by: Mans Rullgard <m...@mansr.com>
> Acked-by: Matthias Brugger <matthias@gmail.com>
> Acked-by: Michal Simek <michal.si...@xilinx.com>
> Acked-by: Neil Armstrong <narmstr...@baylibre.com>
> Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
> Acked-by: Thierry Reding <tred...@nvidia.com>
> Acked-by: Tomas Winkler <tomas.wink...@intel.com>
> Acked-by: Patrice Chotard <patrice.chot...@st.com>
> Acked-by: William Breathitt Gray <vilhelm.g...@gmail.com>
> Reviewed-by: Eric Anholt <e...@anholt.net>
> ---
> 
> Notes:
> v4:
>   - Drop coh901327_wdt since it allready is a pending patch
> v3:
>   - Keep license text for ebc-c384_wdt
> v2:
>   - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt
>   - Change to BSD-3-Clause for meson_gxbb_wdt
> v1: Please have an extra look at meson_gxbb_wdt.c
> 

>  drivers/watchdog/ar7_wdt.c | 14 +-

>  drivers/watchdog/bcm2835_wdt.c |  5 +---
>  drivers/watchdog/bcm47xx_wdt.c |  5 +---
>  drivers/watchdog/bcm63xx_wdt.c |  5 +---
>  drivers/watchdog/bcm7038_wdt.c | 12 ++--
>  drivers/watchdog/bcm_kona_wdt.c|  9 +-

>  drivers/watchdog/mtx-1_wdt.c   | 11 +---

For these drivers above:

Acked-by: Florian Fainelli <f.faine...@gmail.com>

and it looks like you missed rdc321x_wdt.c, is there a specific reason?
-- 
Florian


Re: PA Semi PWRficient Gigabit Ethernet doesn't work anymore since the first networking updates for the kernel 4.16

2018-02-04 Thread Florian Fainelli


On 02/04/2018 09:16 AM, Andrew Lunn wrote:
> On Sun, Feb 04, 2018 at 05:47:03PM +0100, Christian Zigotzky wrote:
>> Hello,
>>
>> The PA Semi PWRficient Gigabit Ethernet doesn't work anymore since the first
>> networking updates [1] for the kernel 4.16.
>>
>> Error messages:
>>
>> [    0.634241] libphy: pasemi gpio mdio bus: probed
>> [    0.634749] pasemi gpio mdio bus: Cannot register as MDIO bus, err -38
> 
> -38 is ENOSYS.
> 
>> --- a/drivers/net/phy/mdio_bus.c 2018-02-03 17:34:46.973045321 +0100
>> +++ b/drivers/net/phy/mdio_bus.c 2018-02-04 11:03:14.909093360 +0100
>> @@ -47,41 +47,11 @@
>>  
>>  #include "mdio-boardinfo.h"
>>  
>> -static int mdiobus_register_gpiod(struct mdio_device *mdiodev)
>> -{
>> -struct gpio_desc *gpiod = NULL;
>> -
>> -/* Deassert the optional reset signal */
>> -if (mdiodev->dev.of_node)
>> -gpiod = fwnode_get_named_gpiod(>dev.of_node->fwnode,
>> -   "reset-gpios", 0, GPIOD_OUT_LOW,
>> -   "PHY reset");
> 
> So i think you don't have GPIOLIB enabled. Hence you are hitting
> 
> http://elixir.free-electrons.com/linux/latest/source/include/linux/gpio/consumer.h#L470
> 
> static inline
> struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
>const char *propname, int index,
>enum gpiod_flags dflags,
>const char *label)
> {
>   return ERR_PTR(-ENOSYS);
> }
> 
> So rather than just deleting all this code, breaking other platforms
> that need this gpio, lets try a real fix. Please try this. If it
> works, i will formally submit it.
> 
>Andrew
> 
> From a4210ba306948497d7360927c1e532eb903c58b2 Mon Sep 17 00:00:00 2001
> From: Andrew Lunn <and...@lunn.ch>
> Date: Sun, 4 Feb 2018 11:09:20 -0600
> Subject: [PATCH] net: phy: Handle not having GPIO enabled in the kernel
> 
> If CONFIG_GPIOLIB is disabled, fwnode_get_named_gpiod() becomes a stub
> function, which return -ENOSYS. Handle this in the same way as
> -ENOENT, i.e. assume there is no GPIO used to reset the PHYs.
> 
> Reported-by: Christian Zigotzky <chzigot...@xenosoft.de>
> Signed-off-by: Andrew Lunn <and...@lunn.ch>

Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
Fixes: bafbdd527d56 ("phylib: Add device reset GPIO support")

> ---
>  drivers/net/phy/mdio_bus.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 88272b3ac2e2..24b5511222c8 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -56,7 +56,8 @@ static int mdiobus_register_gpiod(struct mdio_device 
> *mdiodev)
>   gpiod = fwnode_get_named_gpiod(>dev.of_node->fwnode,
>  "reset-gpios", 0, GPIOD_OUT_LOW,
>  "PHY reset");
> - if (PTR_ERR(gpiod) == -ENOENT)
> + if (PTR_ERR(gpiod) == -ENOENT ||
> + PTR_ERR(gpiod) == -ENOSYS)
>   gpiod = NULL;
>   else if (IS_ERR(gpiod))
>   return PTR_ERR(gpiod);
> 

-- 
Florian


[PATCH 10/15] net: fs_enet: Utilize phy_ethtool_nway_reset

2016-11-15 Thread Florian Fainelli
Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 44f50e168703..34843c155420 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -807,11 +807,6 @@ static void fs_get_regs(struct net_device *dev, struct 
ethtool_regs *regs,
regs->version = 0;
 }
 
-static int fs_nway_reset(struct net_device *dev)
-{
-   return 0;
-}
-
 static u32 fs_get_msglevel(struct net_device *dev)
 {
struct fs_enet_private *fep = netdev_priv(dev);
@@ -865,7 +860,7 @@ static int fs_set_tunable(struct net_device *dev,
 static const struct ethtool_ops fs_ethtool_ops = {
.get_drvinfo = fs_get_drvinfo,
.get_regs_len = fs_get_regs_len,
-   .nway_reset = fs_nway_reset,
+   .nway_reset = phy_ethtool_nway_reset,
.get_link = ethtool_op_get_link,
.get_msglevel = fs_get_msglevel,
.set_msglevel = fs_set_msglevel,
-- 
2.9.3



[PATCH 12/15] net: ethernet: ucc: Utilize phy_ethtool_nway_reset

2016-11-15 Thread Florian Fainelli
Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
---
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c 
b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
index 812a968a78e9..8ba636f61b50 100644
--- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
+++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
@@ -332,13 +332,6 @@ static void uec_get_ethtool_stats(struct net_device 
*netdev,
}
 }
 
-static int uec_nway_reset(struct net_device *netdev)
-{
-   struct ucc_geth_private *ugeth = netdev_priv(netdev);
-
-   return phy_start_aneg(ugeth->phydev);
-}
-
 /* Report driver information */
 static void
 uec_get_drvinfo(struct net_device *netdev,
@@ -394,7 +387,7 @@ static const struct ethtool_ops uec_ethtool_ops = {
.get_regs   = uec_get_regs,
.get_msglevel   = uec_get_msglevel,
.set_msglevel   = uec_set_msglevel,
-   .nway_reset = uec_nway_reset,
+   .nway_reset = phy_ethtool_nway_reset,
.get_link   = ethtool_op_get_link,
.get_ringparam  = uec_get_ringparam,
.set_ringparam  = uec_set_ringparam,
-- 
2.9.3



Re: [PATCH 3/3] net: fs_enet: make rx_copybreak value configurable

2016-08-24 Thread Florian Fainelli
On 08/24/2016 03:36 AM, Christophe Leroy wrote:
> Measurement shows that on a MPC8xx running at 132MHz, the optimal
> limit is 112:
> * 114 bytes packets are processed in 147 TB ticks with higher copybreak
> * 114 bytes packets are processed in 148 TB ticks with lower copybreak
> * 128 bytes packets are processed in 154 TB ticks with higher copybreak
> * 128 bytes packets are processed in 148 TB ticks with lower copybreak
> * 238 bytes packets are processed in 172 TB ticks with higher copybreak
> * 238 bytes packets are processed in 148 TB ticks with lower copybreak
> 
> However it might be different on other processors
> and/or frequencies. So it is useful to make it configurable.
> 
> Signed-off-by: Christophe Leroy 
> ---
>  drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 8 +---
>  include/linux/fs_enet_pd.h| 1 -
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
> b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index addcae6..b59bbf8 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> @@ -60,6 +60,10 @@ module_param(fs_enet_debug, int, 0);
>  MODULE_PARM_DESC(fs_enet_debug,
>"Freescale bitmapped debugging message enable value");
>  
> +static int rx_copybreak = 240;
> +module_param(rx_copybreak, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(rx_copybreak, "Receive copy threshold");

There is an ethtool tunable knob for copybreak now, which you should
prefer over a module parameter, see
drivers/net/ethernet/cisco/enic/enic_ethtool.c
-- 
Florian


Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

2016-01-15 Thread Florian Fainelli
Le 15/01/2016 14:57, Sebastian Hesselbarth a écrit :
> On 15.01.2016 05:01, Shaohui Xie wrote:
>>> -Original Message-
>>> From: Andrew Lunn [mailto:and...@lunn.ch]
>>> Sent: Friday, January 15, 2016 12:44 AM
>>> To: shh@gmail.com
>>> Cc: devicet...@vger.kernel.org; net...@vger.kernel.org; linuxppc-
>>> d...@lists.ozlabs.org; f.faine...@gmail.com; da...@davemloft.net; Shaohui 
>>> Xie
>>> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
>>>
>>> On Thu, Jan 14, 2016 at 04:23:59PM +0800, shh@gmail.com wrote:
 From: Shaohui Xie 

 This commit adds necessary definitions for the PHY layer to recognize
 backplane Ethernet 1000BASE-KX and 10GBASE-KR as valid PHY interfaces,
 "1000base-kx" for 1000BASE-KX, "10gbase-kr" for 10GBASE-KR.

 Signed-off-by: Shaohui Xie 
 ---
 changes in v2:
 new patch.
> 
> Shaohui,
> 
> it would be more useful to describe _what_ is new here compared to v1.
> 
> Anyway:
> 
  Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
  include/linux/phy.h| 6 ++
  2 files changed, 8 insertions(+), 2 deletions(-)

 diff --git a/Documentation/devicetree/bindings/net/ethernet.txt
 b/Documentation/devicetree/bindings/net/ethernet.txt
 index 5d88f37..1166a5c 100644
 --- a/Documentation/devicetree/bindings/net/ethernet.txt
 +++ b/Documentation/devicetree/bindings/net/ethernet.txt
 @@ -11,8 +11,8 @@ The following properties are common to the Ethernet
>>> controllers:
the maximum frame size (there's contradiction in ePAPR).
  - phy-mode: string, operation mode of the PHY interface; supported values 
 are
"mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii",
 "rgmii", "rgmii-id",
 -  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a
 de-facto
 -  standard property;
 +  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "1000base-kx",
 + "10gbase-kr";  this is now a de-facto standard property;
>>>
>>> I know very little about this, so i'm just asking a question. None of the 
>>> other
>>> interface modes contain a bit rate. So is the bit rate needed for your two 
>>> new
>>> modes?
>>
>> 1000BASE-KX and 10GBASE-KR are terms in IEEE802.3, so as XGMII and GMII. 
>> There are interfaces could be different bit rates but same types, 
>> e.g. 100BASE-LX10 and 1000BASE-LX10, or 40GBASE-KR4 and 100GBASE-KR4, 
>> having bit rate is clear to represent hardware.
>>
> 
> If you look at the list of possible values for "phy-mode" you'd see that
> none of it describes a PHY-to-PHY connection but all are for MAC-to-PHY
> connections. Also, names above suggest it already: MII is short for
> media _independent_ interface.
> 
> I copy Andrew's concerns and think that neither 1base-kx nor
> 10gbase-kr belong in the list of phy-mode properties.

I concur with that as well, if the phy connection does not really matter
here, or does not seem like a good fit, maybe we should have a different
property, or just define the hardware interface a little differently?
-- 
Florian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 0/9] Phy, mdiobus, and netdev struct device fixes

2015-09-24 Thread Florian Fainelli
On 24/09/15 12:17, Russell King - ARM Linux wrote:
> Hi,
> 
> The third version of this series fixes the build error which David
> identified, and drops the broken changes for the Cavium Thunger BGX
> ethernet driver as this driver requires some complex changes to
> resolve the leakage - and this is best done by people who can test
> the driver.
> 
> Compared to v2, the only patch which has changed is patch 6
>   "net: fix phy refcounting in a bunch of drivers"
> 
> I _think_ I've been able to build-test all the drivers touched by
> that patch to some degree now, though several of them needed the
> Kconfig hacked to allow it (not all had || COMPILE_TEST clause on
> their dependencies.)

Tested-by: Florian Fainelli <f.faine...@gmail.com>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>

Thanks for fixing that.

> 
> Previous cover letters below:
> 
> This is the second version of the series, with the comments David had
> on the first patch fixed up.  Original series description with updated
> diffstat below.
> 
> While looking at the DSA code, I noticed we have a
> of_find_net_device_by_node(), and it looks like users of that are
> similarly buggy - it looks like net/dsa/dsa.c is the only user.  Fix
> that too.
> 
> Hi,
> 
> While looking at the phy code, I identified a number of weaknesses
> where refcounting on device structures was being leaked, where
> modules could be removed while in-use, and where the fixed-phy could
> end up having unintended consequences caused by incorrect calls to
> fixed_phy_update_state().
> 
> This patch series resolves those issues, some of which were discovered
> with testing on an Armada 388 board.  Not all patches are fully tested,
> particularly the one which touches several network drivers.
> 
> When resolving the struct device refcounting problems, several different
> solutions were considered before settling on the implementation here -
> one of the considerations was to avoid touching many network drivers.
> The solution here is:
> 
>   phy_attach*() - takes a refcount
>   phy_detach*() - drops the phy_attach refcount
> 
> Provided drivers always attach and detach their phys, which they should
> already be doing, this should change nothing, even if they leak a refcount.
> 
>   of_phy_find_device() and of_* functions which use that take
>   a refcount.  Arrange for this refcount to be dropped once
>   the phy is attached.
> 
> This is the reason why the previous change is important - we can't drop
> this refcount taken by of_phy_find_device() until something else holds
> a reference on the device.  This resolves the leaked refcount caused by
> using of_phy_connect() or of_phy_attach().
> 
> Even without the above changes, these drivers are leaking by calling
> of_phy_find_device().  These drivers are addressed by adding the
> appropriate release of that refcount.
> 
> The mdiobus code also suffered from the same kind of leak, but thankfully
> this only happened in one place - the mdio-mux code.
> 
> I also found that the try_module_get() in the phy layer code was utterly
> useless: phydev->dev.driver was guaranteed to always be NULL, so
> try_module_get() was always being called with a NULL argument.  I proved
> this with my SFP code, which declares its own MDIO bus - the module use
> count was never incremented irrespective of how I set the MDIO bus up.
> This allowed the MDIO bus code to be removed from the kernel while there
> were still PHYs attached to it.
> 
> One other bug was discovered: while using in-band-status with mvneta, it
> was found that if a real phy is attached with in-band-status enabled,
> and another ethernet interface is using the fixed-phy infrastructure, the
> interface using the fixed-phy infrastructure is configured according to
> the other interface using the in-band-status - which is caused by the
> fixed-phy code not verifying that the phy_device passed in is actually
> a fixed-phy device, rather than a real MDIO phy.
> 
> Lastly, having mdio_bus reversing phy_device_register() internals seems
> like a layering violation - it's trivial to move that code to the phy
> device layer.
> 
>  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 24 ++
>  drivers/net/ethernet/freescale/gianfar.c   |  6 ++-
>  drivers/net/ethernet/freescale/ucc_geth.c  |  8 +++-
>  drivers/net/ethernet/marvell/mvneta.c  |  2 +
>  drivers/net/ethernet/xilinx/xilinx_emaclite.c  |  2 +
>  drivers/net/phy/fixed_phy.c|  2 +-
>  drivers/net/phy/mdio-mux.c | 19 +---
>  drivers/net/phy/mdio_bus.c | 24 ++
>  drivers/net/phy/phy_device.c  

Re: [PATCH 0/7] Phy and mdiobus fixes

2015-09-19 Thread Florian Fainelli
Le 09/18/15 02:46, Russell King - ARM Linux a écrit :
> Hi,
> 
> While looking at the phy code, I identified a number of weaknesses
> where refcounting on device structures was being leaked, where
> modules could be removed while in-use, and where the fixed-phy could
> end up having unintended consequences caused by incorrect calls to
> fixed_phy_update_state().
> 
> This patch series resolves those issues, some of which were discovered
> with testing on an Armada 388 board.  Not all patches are fully tested,
> particularly the one which touches several network drivers.
> 
> When resolving the struct device refcounting problems, several different
> solutions were considered before settling on the implementation here -
> one of the considerations was to avoid touching many network drivers.
> The solution here is:
> 
>   phy_attach*() - takes a refcount
>   phy_detach*() - drops the phy_attach refcount
> 
> Provided drivers always attach and detach their phys, which they should
> already be doing, this should change nothing, even if they leak a refcount.
> 
>   of_phy_find_device() and of_* functions which use that take
>   a refcount.  Arrange for this refcount to be dropped once
>   the phy is attached.
> 
> This is the reason why the previous change is important - we can't drop
> this refcount taken by of_phy_find_device() until something else holds
> a reference on the device.  This resolves the leaked refcount caused by
> using of_phy_connect() or of_phy_attach().
> 
> Even without the above changes, these drivers are leaking by calling
> of_phy_find_device().  These drivers are addressed by adding the
> appropriate release of that refcount.
> 
> The mdiobus code also suffered from the same kind of leak, but thankfully
> this only happened in one place - the mdio-mux code.
> 
> I also found that the try_module_get() in the phy layer code was utterly
> useless: phydev->dev.driver was guaranteed to always be NULL, so
> try_module_get() was always being called with a NULL argument.  I proved
> this with my SFP code, which declares its own MDIO bus - the module use
> count was never incremented irrespective of how I set the MDIO bus up.
> This allowed the MDIO bus code to be removed from the kernel while there
> were still PHYs attached to it.
> 
> One other bug was discovered: while using in-band-status with mvneta, it
> was found that if a real phy is attached with in-band-status enabled,
> and another ethernet interface is using the fixed-phy infrastructure, the
> interface using the fixed-phy infrastructure is configured according to
> the other interface using the in-band-status - which is caused by the
> fixed-phy code not verifying that the phy_device passed in is actually
> a fixed-phy device, rather than a real MDIO phy.
> 
> Lastly, having mdio_bus reversing phy_device_register() internals seems
> like a layering violation - it's trivial to move that code to the phy
> device layer.

Reviewed-by: Florian Fainelli <f.faine...@gmail.com>

Thanks!
-- 
Florian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [v2 2/9] dpaa_eth: add support for DPAA Ethernet

2015-08-08 Thread Florian Fainelli
Le 08/05/15 08:41, Madalin Bucur a écrit :
 This introduces the Freescale Data Path Acceleration Architecture
 (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan,
 BMan, PAMU and FMan drivers to deliver Ethernet connectivity on
 the Freescale DPAA QorIQ platforms.
 
 Signed-off-by: Madalin Bucur madalin.bu...@freescale.com
 ---
[snip]
 +
 +if FSL_DPAA_ETH
 +
 +config FSL_DPAA_CS_THRESHOLD_1G
 + hex Egress congestion threshold on 1G ports
 + range 0x1000 0x1000
 + default 0x0600

This sounds like something you would want to be able to configure at
runtime, either via private sysfs attributes, or better, using ethtool
and either a newly introduced set of tunables, or creating a private
driver API for this.

 + ---help---
 +   The size in bytes of the egress Congestion State notification 
 threshold on 1G ports.
 +   The 1G dTSECs can quite easily be flooded by cores doing Tx in a 
 tight loop
 +   (e.g. by sending UDP datagrams at while(1) speed),
 +   and the larger the frame size, the more acute the problem.
 +   So we have to find a balance between these factors:
 +- avoiding the device staying congested for a prolonged time 
 (risking
 + the netdev watchdog to fire - see also the tx_timeout 
 module param);
 +   - affecting performance of protocols such as TCP, which 
 otherwise
 +  behave well under the congestion notification mechanism;
 +- preventing the Tx cores from tightly-looping (as if the 
 congestion
 +  threshold was too low to be effective);
 +- running out of memory if the CS threshold is set too high.
 +
 +config FSL_DPAA_CS_THRESHOLD_10G
 + hex Egress congestion threshold on 10G ports
 + range 0x1000 0x2000
 + default 0x1000
 + ---help ---
 +   The size in bytes of the egress Congestion State notification 
 threshold on 10G ports.
 +
 +config FSL_DPAA_INGRESS_CS_THRESHOLD
 + hex Ingress congestion threshold on FMan ports
 + default 0x1000
 + ---help---
 +   The size in bytes of the ingress tail-drop threshold on FMan ports.
 +   Traffic piling up above this value will be rejected by QMan and 
 discarded by FMan.

Same here.
-- 
Florian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: net: ucc: tbi phy detection broken by 058112c7efc9ef43bb511c137293dddbe6e42908

2014-12-20 Thread Florian Fainelli
2014-12-18 19:49 GMT-08:00 Lennart Sorensen lsore...@csclub.uwaterloo.ca:
 I have been trying to move an 8360 based system from a 3.0 kernel to a
 3.12 (on the way to 3.14 with ipipe/xenomai) kernel and encountered an
 oops in the ucc_geth driver when using RTBI mode on one of the ucc
 ports.  I haven't managed to find any commits to of_mdio or ucc_geth or
 fsl_pq_mdio that would appear to address this problem, so I believe it
 is still present in the latest kernel, but have not confirmed that with
 testing yet.

 Commit 058112c7efc9ef43bb511c137293dddbe6e42908 appears to have broken
 ucc support for tbi phy detection.

 With the patch in place, I am unable to get the mdio bus to create phy
 devices for the tbi phy in the ucc on an 8360e, and the ucc_geth driver
 causes a kernel oops, while with the patch reverted, it does create them
 and the driver comes up and works.

 The tbi phy is needed when using a ucc in RTBI, TBI or SGMII mode.

 I am not convinced that the tbi phy really behaves quite like a real phy,
 which may be why get_phy_device does not work with it.  Perhaps there
 is a better way to deal with the tbi phy on the ucc for this purpose.

There are some comments in ucc_geth that also lead me to believe this
is a just a hack instead of a real Ethernet PHY device. Part of what I
think got broken is because of this comment:

/* Initialize TBI PHY interface for communicating with the
 * SERDES lynx PHY on the chip.  We communicate with this PHY
 * through the MDIO bus on each controller, treating it as a
 * normal PHY at the address found in the UTBIPA register.  We assume
 * that the UTBIPA register is valid.  Either the MDIO bus code will set
 * it to a value that doesn't conflict with other PHYs on the bus, or the
 * value doesn't matter, as there are no other PHYs on the bus.
 */

In particular this one:

Either the MDIO bus code will set
 * it to a value that doesn't conflict with other PHYs on the bus, or the
 * value doesn't matter, as there are no other PHYs on the bus.

and what Sebastian removed did exactly that, we used the special MDIO
broadcast address 0 to provide this whatever. If this is such a
requirement from the ucc_geth driver and TBI PHYs, maybe we should
have this hack somewhere in the actual MDIO driver used by the
ucc_geth driver instead, or set a flag/read the PHY connection mode
and do this in drivers/of/of_mdio.c


 Certainly as it is, this patch has caused a regression though, although
 probably not very many systems with ucc ports actually use one of the
 affected modes so the damage isn't that great.

 --
 Len Sorensen



-- 
Florian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 0/9] net: of_phy_connect_fixed_link removal

2014-05-22 Thread Florian Fainelli
Hi all,

This patch set removes of_phy_connect_fixed_link() from the tree now that
we have a better solution for dealing with fixed PHY (emulated PHY) devices
for drivers that require them.

First two patches update the 'fixed-link' Device Tree binding and drivers to
refere to it.

Patches 3 to 7 update the in-tree network drivers that use
of_phy_connect_fixed_link()

Patch 8 removes of_phy_connect_fixed_link

Patch 9 removes the PowerPC code that parsed the 'fixed-link' property.

Patch 9 can be merged via the net-next tree if the PowerPC folks ack it,
but it really has to be merged after the first 8 patches in order to avoid
breakage.

Florian Fainelli (9):
  Documentation: devicetree: add old and deprecated 'fixed-link'
  Documentation: devicetree: net: refer to fixed-link.txt
  net: bcmgenet: use the new fixed PHY helpers
  net: systemport: use the new fixed PHY helpers
  fs_enet: use the new fixed PHY helpers
  gianfar: use the new fixed PHY helpers
  ucc_geth: use the new fixed PHY helpers
  of: mdio: remove of_phy_connect_fixed_link
  powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code

 .../devicetree/bindings/net/broadcom-bcmgenet.txt  |  2 +-
 .../bindings/net/broadcom-systemport.txt   |  2 +-
 .../devicetree/bindings/net/fixed-link.txt | 12 +++
 .../devicetree/bindings/net/fsl-tsec-phy.txt   |  5 +--
 arch/powerpc/sysdev/fsl_soc.c  | 32 --
 drivers/net/ethernet/broadcom/bcmsysport.c | 17 --
 drivers/net/ethernet/broadcom/bcmsysport.h |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 21 +++-
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 17 ++
 drivers/net/ethernet/freescale/gianfar.c   | 14 ++--
 drivers/net/ethernet/freescale/ucc_geth.c  | 14 ++--
 drivers/of/of_mdio.c   | 38 --
 include/linux/of_mdio.h| 10 --
 13 files changed, 76 insertions(+), 109 deletions(-)

-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 1/9] Documentation: devicetree: add old and deprecated 'fixed-link'

2014-05-22 Thread Florian Fainelli
Update the fixed-link Device Tree binding documentation to contain
information about the old and deprecated 5-digit 'fixed-link' property.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
Changes in v2:
- fixed typos and spelling mistakes as spotted by Sergei

 Documentation/devicetree/bindings/net/fixed-link.txt | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt 
b/Documentation/devicetree/bindings/net/fixed-link.txt
index e956de1be935..82bf7e0f47b6 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -18,6 +18,18 @@ properties:
 * 'asym-pause' (boolean, optional), to indicate that asym_pause should
   be enabled.
 
+Old, deprecated 'fixed-link' binding:
+
+* A 'fixed-link' property in the Ethernet MAC node, with 5 cells, of the
+  form a b c d e with the following accepted values:
+  - a: emulated PHY ID, choose any but but unique to the all specified
+fixed-links, from 0 to 31
+  - b: duplex configuration: 0 for half duplex, 1 for full duplex
+  - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
+  - d: pause configuration: 0 for no pause, 1 for pause
+  - e: asymmetric pause configuration: 0 for no asymmetric pause, 1 for
+asymmetric pause
+
 Example:
 
 ethernet@0 {
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 2/9] Documentation: devicetree: net: refer to fixed-link.txt

2014-05-22 Thread Florian Fainelli
Update the Freescale TSEC PHY, Broadcom GENET  SYSTEMPORT Device Tree
binding documentation to refer to the fixed-link Device Tree binding in
fixed-link.txt.

Reviewed-by: Thomas Petazzoni thomas.petazz...@free-electrons.com
Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
No changes in v2

 Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt   | 2 +-
 Documentation/devicetree/bindings/net/broadcom-systemport.txt | 2 +-
 Documentation/devicetree/bindings/net/fsl-tsec-phy.txt| 5 +
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt 
b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
index f2febb94550e..451fef26b4df 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -24,7 +24,7 @@ Optional properties:
 - fixed-link: When the GENET interface is connected to a MoCA hardware block or
   when operating in a RGMII to RGMII type of connection, or when the MDIO bus 
is
   voluntarily disabled, this property should be used to describe the fixed 
link.
-  See Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for information on
+  See Documentation/devicetree/bindings/net/fixed-link.txt for information on
   the property specifics
 
 Required child nodes:
diff --git a/Documentation/devicetree/bindings/net/broadcom-systemport.txt 
b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
index 1b7600e022dd..c183ea90d9bc 100644
--- a/Documentation/devicetree/bindings/net/broadcom-systemport.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
@@ -8,7 +8,7 @@ Required properties:
 - local-mac-address: Ethernet MAC address (48 bits) of this adapter
 - phy-mode: Should be a string describing the PHY interface to the
   Ethernet switch/PHY, see Documentation/devicetree/bindings/net/ethernet.txt
-- fixed-link: see Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for
+- fixed-link: see Documentation/devicetree/bindings/net/fixed-link.txt for
   the property specific details
 
 Optional properties:
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt 
b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 737cdef4f903..be6ea8960f20 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -42,10 +42,7 @@ Properties:
 interrupt.  For TSEC and eTSEC devices, the first interrupt is
 transmit, the second is receive, and the third is error.
   - phy-handle : See ethernet.txt file in the same directory.
-  - fixed-link : a b c d e where a is emulated phy id - choose any,
-but unique to the all specified fixed-links, b is duplex - 0 half,
-1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
-pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
+  - fixed-link : See fixed-link.txt in the same directory.
   - phy-connection-type : See ethernet.txt file in the same directory.
 This property is only really needed if the connection is of type
 rgmii-id, as all other connection types are detected by hardware.
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 3/9] net: bcmgenet: use the new fixed PHY helpers

2014-05-22 Thread Florian Fainelli
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
No changes in v2

 drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c 
b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 4608673beaff..add8d8596084 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -298,6 +298,7 @@ int bcmgenet_mii_config(struct net_device *dev)
 static int bcmgenet_mii_probe(struct net_device *dev)
 {
struct bcmgenet_priv *priv = netdev_priv(dev);
+   struct device_node *dn = priv-pdev-dev.of_node;
struct phy_device *phydev;
unsigned int phy_flags;
int ret;
@@ -307,15 +308,19 @@ static int bcmgenet_mii_probe(struct net_device *dev)
return 0;
}
 
-   if (priv-phy_dn)
-   phydev = of_phy_connect(dev, priv-phy_dn,
-   bcmgenet_mii_setup, 0,
-   priv-phy_interface);
-   else
-   phydev = of_phy_connect_fixed_link(dev,
-   bcmgenet_mii_setup,
-   priv-phy_interface);
+   /* In the case of a fixed PHY, the DT node associated
+* to the PHY is the Ethernet MAC DT node.
+*/
+   if (of_phy_is_fixed_link(dn)) {
+   ret = of_phy_register_fixed_link(dn);
+   if (ret)
+   return ret;
+
+   priv-phy_dn = dn;
+   }
 
+   phydev = of_phy_connect(dev, priv-phy_dn, bcmgenet_mii_setup, 0,
+   priv-phy_interface);
if (!phydev) {
pr_err(could not attach to PHY\n);
return -ENODEV;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 4/9] net: systemport: use the new fixed PHY helpers

2014-05-22 Thread Florian Fainelli
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
No changes in v2

 drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++--
 drivers/net/ethernet/broadcom/bcmsysport.h |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c 
b/drivers/net/ethernet/broadcom/bcmsysport.c
index d40c5b969e9e..dc708a888f80 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
/* Read CRC forward */
priv-crc_fwd = !!(umac_readl(priv, UMAC_CMD)  CMD_CRC_FWD);
 
-   priv-phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
-   priv-phy_interface);
+   priv-phydev = of_phy_connect(dev, priv-phy_dn, bcm_sysport_adj_link,
+   0, priv-phy_interface);
if (!priv-phydev) {
netdev_err(dev, could not attach to PHY\n);
return -ENODEV;
@@ -1551,6 +1551,19 @@ static int bcm_sysport_probe(struct platform_device 
*pdev)
if (priv-phy_interface  0)
priv-phy_interface = PHY_INTERFACE_MODE_GMII;
 
+   /* In the case of a fixed PHY, the DT node associated
+* to the PHY is the Ethernet MAC DT node.
+*/
+   if (of_phy_is_fixed_link(dn)) {
+   ret = of_phy_register_fixed_link(dn);
+   if (ret) {
+   dev_err(pdev-dev, failed to register fixed PHY\n);
+   goto err;
+   }
+
+   priv-phy_dn = dn;
+   }
+
/* Initialize netdevice members */
macaddr = of_get_mac_address(dn);
if (!macaddr || !is_valid_ether_addr(macaddr)) {
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h 
b/drivers/net/ethernet/broadcom/bcmsysport.h
index abdeb62616df..73fd04a94797 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -656,6 +656,7 @@ struct bcm_sysport_priv {
unsigned intrx_c_index;
 
/* PHY device */
+   struct device_node  *phy_dn;
struct phy_device   *phydev;
phy_interface_t phy_interface;
int old_pause;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 5/9] fs_enet: use the new fixed PHY helpers

2014-05-22 Thread Florian Fainelli
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
- merge if conditions on the same line as suggested by Sergei
- add comment explaining the device_node pointer assignment

 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index dc80db41d6b3..cfaf17b70f3f 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -792,10 +792,6 @@ static int fs_init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, fep-fpi-phy_node, fs_adjust_link, 0,
iface);
if (!phydev) {
-   phydev = of_phy_connect_fixed_link(dev, fs_adjust_link,
-  iface);
-   }
-   if (!phydev) {
dev_err(dev-dev, Could not attach to PHY\n);
return -ENODEV;
}
@@ -1029,9 +1025,16 @@ static int fs_enet_probe(struct platform_device *ofdev)
fpi-use_napi = 1;
fpi-napi_weight = 17;
fpi-phy_node = of_parse_phandle(ofdev-dev.of_node, phy-handle, 0);
-   if ((!fpi-phy_node)  (!of_get_property(ofdev-dev.of_node, 
fixed-link,
- NULL)))
-   goto out_free_fpi;
+   if (!fpi-phy_node  of_phy_is_fixed_link(ofdev-dev.of_node)) {
+   err = of_phy_register_fixed_link(ofdev-dev.of_node);
+   if (err)
+   goto out_free_fpi;
+
+   /* In the case of a fixed PHY, the DT node associated
+* to the PHY is the Ethernet MAC DT node.
+*/
+   fpi-phy_node = ofdev-dev.of_node;
+   }
 
if (of_device_is_compatible(ofdev-dev.of_node, fsl,mpc5125-fec)) {
phy_connection_type = of_get_property(ofdev-dev.of_node,
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 6/9] gianfar: use the new fixed PHY helpers

2014-05-22 Thread Florian Fainelli
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
No changes in v2

 drivers/net/ethernet/freescale/gianfar.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c 
b/drivers/net/ethernet/freescale/gianfar.c
index e2d42475b006..282674027c92 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -889,6 +889,17 @@ static int gfar_of_init(struct platform_device *ofdev, 
struct net_device **pdev)
 
priv-phy_node = of_parse_phandle(np, phy-handle, 0);
 
+   /* In the case of a fixed PHY, the DT node associated
+* to the PHY is the Ethernet MAC DT node.
+*/
+   if (of_phy_is_fixed_link(np)) {
+   err = of_phy_register_fixed_link(np);
+   if (err)
+   goto err_grp_init;
+
+   priv-phy_node = np;
+   }
+
/* Find the TBI PHY.  If it's not there, we don't support SGMII */
priv-tbi_node = of_parse_phandle(np, tbi-handle, 0);
 
@@ -1660,9 +1671,6 @@ static int init_phy(struct net_device *dev)
 
priv-phydev = of_phy_connect(dev, priv-phy_node, adjust_link, 0,
  interface);
-   if (!priv-phydev)
-   priv-phydev = of_phy_connect_fixed_link(dev, adjust_link,
-interface);
if (!priv-phydev) {
dev_err(dev-dev, could not attach to PHY\n);
return -ENODEV;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 7/9] ucc_geth: use the new fixed PHY helpers

2014-05-22 Thread Florian Fainelli
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
No changes in v2

 drivers/net/ethernet/freescale/ucc_geth.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index c8299c31b21f..fab39e295441 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1728,9 +1728,6 @@ static int init_phy(struct net_device *dev)
 
phydev = of_phy_connect(dev, ug_info-phy_node, adjust_link, 0,
priv-phy_interface);
-   if (!phydev)
-   phydev = of_phy_connect_fixed_link(dev, adjust_link,
-  priv-phy_interface);
if (!phydev) {
dev_err(dev-dev, Could not attach to PHY\n);
return -ENODEV;
@@ -3790,6 +3787,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
ug_info-uf_info.irq = irq_of_parse_and_map(np, 0);
 
ug_info-phy_node = of_parse_phandle(np, phy-handle, 0);
+   if (!ug_info-phy_node) {
+   /* In the case of a fixed PHY, the DT node associated
+* to the PHY is the Ethernet MAC DT node.
+*/
+   if (of_phy_is_fixed_link(np)) {
+   err = of_phy_register_fixed_link(np);
+   if (err)
+   return err;
+   }
+   ug_info-phy_node = np;
+   }
 
/* Find the TBI PHY node.  If it's not there, we don't support SGMII */
ug_info-tbi_node = of_parse_phandle(np, tbi-handle, 0);
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 8/9] of: mdio: remove of_phy_connect_fixed_link

2014-05-22 Thread Florian Fainelli
All in-tree drivers have been converted to use the new pair of
functions: of_is_fixed_phy_link() plus of_phy_register_fixed_link(), we
can now safely remove of_phy_connect_fixed_link.

Signed-off-by: Florian Fainelli f.faine...@gmail.com
---
No changes in v2

 drivers/of/of_mdio.c| 38 --
 include/linux/of_mdio.h | 10 --
 2 files changed, 48 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1def0bb5cb37..4c1e01ed16dc 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -246,44 +246,6 @@ struct phy_device *of_phy_connect(struct net_device *dev,
 EXPORT_SYMBOL(of_phy_connect);
 
 /**
- * of_phy_connect_fixed_link - Parse fixed-link property and return a dummy phy
- * @dev: pointer to net_device claiming the phy
- * @hndlr: Link state callback for the network device
- * @iface: PHY data interface type
- *
- * This function is a temporary stop-gap and will be removed soon.  It is
- * only to support the fs_enet, ucc_geth and gianfar Ethernet drivers.  Do
- * not call this function from new drivers.
- */
-struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
-void (*hndlr)(struct net_device *),
-phy_interface_t iface)
-{
-   struct device_node *net_np;
-   char bus_id[MII_BUS_ID_SIZE + 3];
-   struct phy_device *phy;
-   const __be32 *phy_id;
-   int sz;
-
-   if (!dev-dev.parent)
-   return NULL;
-
-   net_np = dev-dev.parent-of_node;
-   if (!net_np)
-   return NULL;
-
-   phy_id = of_get_property(net_np, fixed-link, sz);
-   if (!phy_id || sz  sizeof(*phy_id))
-   return NULL;
-
-   sprintf(bus_id, PHY_ID_FMT, fixed-0, be32_to_cpu(phy_id[0]));
-
-   phy = phy_connect(dev, bus_id, hndlr, iface);
-   return IS_ERR(phy) ? NULL : phy;
-}
-EXPORT_SYMBOL(of_phy_connect_fixed_link);
-
-/**
  * of_phy_attach - Attach to a PHY without starting the state machine
  * @dev: pointer to net_device claiming the phy
  * @phy_np: Node pointer for the PHY
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 0aa367e316cb..d449018d0726 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -22,9 +22,6 @@ extern struct phy_device *of_phy_connect(struct net_device 
*dev,
 struct phy_device *of_phy_attach(struct net_device *dev,
 struct device_node *phy_np, u32 flags,
 phy_interface_t iface);
-extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
-void (*hndlr)(struct net_device *),
-phy_interface_t iface);
 
 extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
 
@@ -59,13 +56,6 @@ static inline struct phy_device *of_phy_attach(struct 
net_device *dev,
return NULL;
 }
 
-static inline struct phy_device *of_phy_connect_fixed_link(struct net_device 
*dev,
-  void (*hndlr)(struct 
net_device *),
-  phy_interface_t 
iface)
-{
-   return NULL;
-}
-
 static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
 {
return NULL;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

  1   2   >