Re: [PATCH v3 4/4] Documentation/powerpc/fadump: add additional parameter feature details
Hi Sourabh, Thanks for updating the documentation.. On 23/01/25 5:12 pm, Sourabh Jain wrote: Update the fadump document to include details about the fadump additional parameter feature. The document includes the following: - Significance of the feature - How to use it - Feature restrictions No functional changes are introduced. Cc: Avnish Chouhan Cc: Brian King Cc: Hari Bathini Cc: Madhavan Srinivasan Cc: Michael Ellerman Cc: Mahesh Salgaonkar Signed-off-by: Sourabh Jain --- Documentation/ABI/testing/sysfs-kernel-fadump | 3 ++- .../arch/powerpc/firmware-assisted-dump.rst | 22 +++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Documentation/ABI/testing/sysfs-kernel-fadump b/Documentation/ABI/testing/sysfs-kernel-fadump index 2f9daa7ca55b..b64b7622e6fc 100644 --- a/Documentation/ABI/testing/sysfs-kernel-fadump +++ b/Documentation/ABI/testing/sysfs-kernel-fadump @@ -55,4 +55,5 @@ Date: May 2024 Contact: linuxppc-dev@lists.ozlabs.org Description: read/write This is a special sysfs file available to setup additional - parameters to be passed to capture kernel. + parameters to be passed to capture kernel. For HASH MMU it + is exported only if RMA size higher than 768MB. diff --git a/Documentation/arch/powerpc/firmware-assisted-dump.rst b/Documentation/arch/powerpc/firmware-assisted-dump.rst index 7e37aadd1f77..7e266e749cd5 100644 --- a/Documentation/arch/powerpc/firmware-assisted-dump.rst +++ b/Documentation/arch/powerpc/firmware-assisted-dump.rst @@ -120,6 +120,28 @@ to ensure that crash data is preserved to process later. e.g. # echo 1 > /sys/firmware/opal/mpipl/release_core > +-- Support for Additional Kernel Arguments in Fadump "Support to add additional kernel parameters to FADump capture kernel" ? + Fadump has a feature that allows passing additional kernel arguments + to the fadump kernel. This feature was primarily designed to disable + kernel functionalities that are not required for the fadump kernel + and to reduce its memory footprint while collecting the dump. The below details are more appropriate in "Sysfs/debugfs files:" section with the details going under /sys/kernel/fadump/bootargs_append: + + Command to Add Additional Kernel Parameters to Fadump: Do those words really need to start with Uppercase? + e.g. + # echo "nr_cpus=16" > /sys/kernel/fadump/bootargs_append + + The above command is sufficient to add additional arguments to fadump. + An explicit service restart is not required. + + Command to Retrieve the Additional Fadump Arguments: + e.g. + # cat /sys/kernel/fadump/bootargs_append + +Note: Additional kernel arguments for fadump with HASH MMU is only + supported if the RMA size is greater than 768 MB. If the RMA + size is less than 768 MB, the kernel does not export the + /sys/kernel/fadump/bootargs_append sysfs node. + - Hari
Re: [PATCH 2/3] bitmap: convert self-test to KUnit
On Fri, 7 Feb 2025 at 21:14, Tamir Duberstein wrote: > Convert the bitmap() self-test to a KUnit test. > > In the interest of keeping the patch reasonably-sized this doesn't > refactor the tests into proper parameterized tests - it's all one big > test case. > > Signed-off-by: Tamir Duberstein > arch/m68k/configs/amiga_defconfig | 1 - > arch/m68k/configs/apollo_defconfig| 1 - > arch/m68k/configs/atari_defconfig | 1 - > arch/m68k/configs/bvme6000_defconfig | 1 - > arch/m68k/configs/hp300_defconfig | 1 - > arch/m68k/configs/mac_defconfig | 1 - > arch/m68k/configs/multi_defconfig | 1 - > arch/m68k/configs/mvme147_defconfig | 1 - > arch/m68k/configs/mvme16x_defconfig | 1 - > arch/m68k/configs/q40_defconfig | 1 - > arch/m68k/configs/sun3_defconfig | 1 - > arch/m68k/configs/sun3x_defconfig | 1 - Acked-by: Geert Uytterhoeven # m68k Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 0/3] bitmap: convert self-test to KUnit
Hi Yuri, On Sat, 8 Feb 2025 at 18:53, Yury Norov wrote: > On Fri, Feb 07, 2025 at 03:14:01PM -0500, Tamir Duberstein wrote: > > On 7/27/24 12:35 AM, Shuah Khan wrote: > > > Please make sure you aren't taking away the ability to run these tests > > > during > > > boot. > > > > > > It doesn't make sense to convert every single test especially when it > > > is intended to be run during boot without dependencies - not as a kunit > > > test > > > but a regression test during boot. > > > > > > bitmap is one example - pay attention to the config help test - bitmap > > > one clearly states it runs regression testing during boot. Any test that > > > says that isn't a candidate for conversion. > > > > > > I am going to nack any such conversions. > > > > The crux of the argument seems to be that the config help text is taken > > to describe the author's intent with the fragment "at boot". I think IMO, "at boot" is a misnomer, as most tests can be either builtin or modular. > KUNIT is disabled in defconfig, at least on x86_64. It is also disabled > on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to I think distros should start setting CONFIG_KUNIT=m. > boot-test bitmaps. Even worse, I'll be unable to build the standalone > test from sources as a module and load it later. If you could build the standalone test from sources as a module, surely you can build the converted standalone test and KUNIT itself as modules, and load both of them later? > Or I misunderstand it, and there's a way to build some particular KUNIT > test without enabling KUNIT in config and/or re-compiling the whole kernel? > Please teach me, if so > > Unless you give me a way to build and run the test in true > production environment, I'm not going with KUNITs. Sorry. FTR, this is why I've been advocating for making all tests modular, and for not letting any test select (possibly unwanted) extra functionality. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 09/12] dt-bindings: memory-controllers: Convert fsl,elbc to YAML
On Sun, Feb 09, 2025 at 02:31:35PM -0600, Crystal Wood wrote: > On Fri, Feb 07, 2025 at 10:30:26PM +0100, J. Neuschäfer via B4 Relay wrote: > > +simple-periph@2,0 { > > +compatible = "fsl,elbc-gpcm-uio"; > > +reg = <0x2 0x0 0x1>; > > +elbc-gpcm-br = <0xfd810800>; > > +elbc-gpcm-or = <0x09f7>; > > +}; > > I know this isn't new, but... since we're using this as an example, > where is the documentation for this fsl,elbc-gpcm-uio and > elbc-gpcm-br/or? What exactly is a simple-periph? > > There are no in-tree device trees that use this either. The bcsr > node was actually a much more normal example, despite that particular > platform having been removed. There are other bcsr nodes that still > exist that could be used instead. OK, I noticed patch 10 after I sent this :-P Seems I didn't like it too much when it was new either: https://lkml.org/lkml/2014/12/9/530 And it's still a bad example for how GPCM devices on this bus should normally be represented. -Crystal
Re: [PATCH v2 09/12] dt-bindings: memory-controllers: Convert fsl,elbc to YAML
On Fri, Feb 07, 2025 at 10:30:26PM +0100, J. Neuschäfer via B4 Relay wrote: > From: "J. Neuschäfer" > > Convert the Freescale localbus controller bindings from text form to > YAML. The updated list of compatible strings reflects current usage > in arch/powerpc/boot/dts/, except that many existing device trees > erroneously specify "simple-bus" in addition to fsl,*elbc. > > Changes compared to the txt version: > - removed the board-control (fsl,mpc8272ads-bcsr) node because it only >appears in this example and nowhere else > - added a new example with NAND flash > - updated list of compatible strings > > Signed-off-by: J. Neuschäfer > --- > > V2: > - fix order of properties in examples, according to dts coding style > - move to Documentation/devicetree/bindings/memory-controllers > - clarify the commit message a tiny bit > - remove unnecessary multiline markers (|) > - define address format in patternProperties > - trim subject line (remove "binding") > - remove use of "simple-bus", because it's technically incorrect While I admit I haven't been following recent developments in this area, as someone who was involved when "simple-bus" was created (and was on the ePAPR committee that standardized it) I'm surprised to hear simple-bus being called "erroneous" or "technically incorrect" here. For non-NAND devices this bus generally meets the definition of "an internal I/O bus that cannot be probed for devices" where "devices on the bus can be accessed directly without additional configuration required". NAND flash is an exception, but those devices have compatibles that are specific to the bus controller. The fact that the address encoding is non-linear is irrelevant; the addresses can still be translated using the standard "ranges" mechanism. This seems to be a disconnect between the schema verification and the way the compatible has previously been defined and used. And as a practical matter, unless I'm missing something (which I might be since I haven't been in devicetree-land for nearly a decade), Linux is relying on simple-bus to probe these devices. There is a driver that binds to the bus itself but that is just for error interrupts and NAND. You'd probably need something like commit 3e25f800afb82bd9e5f8 ("memory: fsl_ifc: populate child devices without relying on simple-bus") and the subsequent fix in dd8adc713b1656 ("memory: fsl_ifc: populate child nodes of buses and mfd devices")... I'm curious what the reasoning was for removing simple-bus from IFC. It seems that the schema verification also played a role in that: https://www.spinics.net/lists/devicetree/msg220418.html ...but there's also the comment in 985ede63a045eabf3f9d ("dt-bindings: memory: fsl: convert ifc binding to yaml schema") that "this will help to enforce the correct probe order between parent device and child devices", but was that really not already guaranteed by the parent/child relationship (and again, it should only really matter for NAND except for the possibility of missing error reports during early boot)? > + compatible: > +oneOf: > + - items: > + - enum: > + - fsl,mpc8313-elbc > + - fsl,mpc8315-elbc > + - fsl,mpc8377-elbc > + - fsl,mpc8378-elbc > + - fsl,mpc8379-elbc > + - fsl,mpc8536-elbc > + - fsl,mpc8569-elbc > + - fsl,mpc8572-elbc > + - fsl,p1020-elbc > + - fsl,p1021-elbc > + - fsl,p1023-elbc > + - fsl,p2020-elbc > + - fsl,p2041-elbc > + - fsl,p3041-elbc > + - fsl,p4080-elbc > + - fsl,p5020-elbc > + - fsl,p5040-elbc > + - const: fsl,elbc Is it really necessary to list every single chip? And then it would need to be updated when new ones came out? I know this particular line of chips is not going to see any new members at this point, but as far as the general approach goes... Does the schema validation complain if it sees an extra compatible it doesn't recognize? If so that's obnoxious. > +examples: > + - | > +localbus@f0010100 { > +compatible = "fsl,mpc8272-localbus", > + "fsl,pq2-localbus"; > +reg = <0xf0010100 0x40>; > +ranges = <0x0 0x0 0xfe00 0x0200 > + 0x1 0x0 0xf450 0x8000 > + 0x2 0x0 0xfd81 0x0001>; > +#address-cells = <2>; > +#size-cells = <1>; > + > +flash@0,0 { > +compatible = "jedec-flash"; > +reg = <0x0 0x0 0x200>; > +bank-width = <4>; > +device-width = <1>; > +}; > + > +simple-periph@2,0 { > +compatible = "fsl,elbc-gpcm-uio"; > +reg = <0x2 0x0 0x1>; > +elbc-gpcm-br = <0xfd810800>; > +elbc-gpcm-or = <0x09f7>; > +}; I know this isn't new, but... since we're using this as an examp
[PATCH] powerpc/fsl_lbc: Explicitly populate bus
From: "J. Neuschäfer" Historically, devicetree nodes representing the Freescale Enhanced Local Bus Controller (eLBC) have compatible strings such as: compatible = "fsl,mpc8313-elbc", "fsl,elbc", "simple-bus"; The "simple-bus" string causes the bus to be populated, and the memory devices contained within it to be discovered. The eLBC bus (as represented in device trees) differs from a simple-bus in a few ways, though: - Addresses are not simple/linear: The first cell of an address is a chip select, the second is an linear address within the space thus selected. Representing 1,0 as 1, for example, would decrease readability[1]. - It is expected that the devices on a simple-bus "can be accessed directly without additional configuration required"[2], but the eLBC needs some configuration. To accommodate devicetrees that declare an eLBC without "simple-bus", explicitly populate the bus in the eLBC driver. [1]: dtc makes such a suggestion opon encountering an eLBC [2]: Quoting the Devicetree Specification Release v0.3 Signed-off-by: J. Neuschäfer --- arch/powerpc/sysdev/fsl_lbc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c index 217cea150987df1e1b5c6dbf9e9a1607dd5ce49b..2007ced997fcf0c7059e5b780893b530764dc8b2 100644 --- a/arch/powerpc/sysdev/fsl_lbc.c +++ b/arch/powerpc/sysdev/fsl_lbc.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -336,7 +337,7 @@ static int fsl_lbc_ctrl_probe(struct platform_device *dev) /* Enable interrupts for any detected events */ out_be32(&fsl_lbc_ctrl_dev->regs->lteir, LTEIR_ENABLE); - return 0; + return devm_of_platform_populate(&dev->dev); err1: free_irq(fsl_lbc_ctrl_dev->irq[0], fsl_lbc_ctrl_dev); --- base-commit: 7ccde445dddcca030cd6ed66974bb80915ad9dd5 change-id: 20250209-localbus-94a6ebb04c12 Best regards, -- J. Neuschäfer
Re: (subset) [PATCH v2 0/2] Remove cxl and cxlflash drivers
On Mon, 03 Feb 2025 18:27:58 +1100, Andrew Donnellan wrote: > This series removes the cxl and cxlflash drivers for IBM CAPI devices. > > CAPI devices have been out of production for some time, and we're not > aware of any remaining users who are likely to want a modern kernel. > There's almost certainly some remaining driver bugs and we don't have much > hardware available to properly test the drivers any more. Removing these > drivers will also mean we can get rid of a non-trivial amount of support > code in arch/powerpc. > > [...] Applied to 6.15/scsi-queue, thanks! [1/2] cxlflash: Remove driver https://git.kernel.org/mkp/scsi/c/772ba9b5bd27 -- Martin K. Petersen Oracle Linux Engineering
[linux-next][next-20250207]Observing Kernel Softlock up's while running kselftest
Greetings!!! I am observing kernel soft lock up's while running kselftest on IBM Power Servers. Though, I colud not reporduce this consistently, but CI has detected this error twice now. Hence reporting. This error was reported firat time, while running signal component tests and second time while running EEH component. linux-next/tools/testing/selftests/powerpc/signal linux-next/tools/testing/selftests/powerpc/eeh Traces: [11480.019928] watchdog: BUG: soft lockup - CPU#0 stuck for 26s! [swapper/0:0] [11480.019935] Modules linked in: nvram(E) rpadlpar_io(E) rpaphp(E) dm_mod(E) bonding(E) tls(E) nft_fib_inet(E) nft_fib_ipv4(E) nft_fib_ipv6(E) nft_fib(E) nft_reject_inet(E) nf_reject_ipv4(E) nf_reject_ipv6(E) nft_reject(E) nft_ct(E) nft_chain_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) rfkill(E) ip_set(E) nf_tables(E) nfnetlink(E) hvcs(E) pseries_rng(E) hvcserver(E) vmx_crypto(E) drm(E) drm_panel_orientation_quirks(E) xfs(E) lpfc(E) sr_mod(E) sd_mod(E) cdrom(E) sg(E) nvmet_fc(E) ibmvscsi(E) nvmet(E) ibmveth(E) scsi_transport_srp(E) nvme_fc(E) nvme_fabrics(E) bnx2x(E) nvme_core(E) be2net(E) mdio(E) scsi_transport_fc(E) fuse(E) [last unloaded: test_cpuidle_latency(OE)] [11480.019990] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Kdump: loaded Tainted: G OE 6.14.0-rc1-next-20250207 #1 [11480.019995] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE [11480.019996] Hardware name: IBM,8375-42A POWER9 (architected) 0x4e0202 0xf05 of:IBM,FW950.80 (VL950_131) hv:phyp pSeries [11480.019997] NIP: c003a2d0 LR: c003a644 CTR: c02a912c [11480.02] REGS: c003bb28 TRAP: 0900 Tainted: G OE (6.14.0-rc1-next-20250207) [11480.020002] MSR: 80009033 CR: 22042442 XER: 2004 [11480.020009] CFAR: IRQMASK: 0 [11480.020009] GPR00: c003a644 c003bb00 c1667500 c003baf8 [11480.020009] GPR04: c4062940 c003bd20 0001 c2277ca0 [11480.020009] GPR08: 0003 0049 2000 [11480.020009] GPR12: c02a912c c300 [11480.020009] GPR16: 0001 0082 0001 0100 [11480.020009] GPR20: 0422 000100110511 [11480.020009] GPR24: 7fff 0001 0003bd5a [11480.020009] GPR28: 0002 0003 fcff fcff [11480.020036] NIP [c003a2d0] __replay_soft_interrupts+0x5c/0x22c [11480.020048] LR [c003a644] arch_local_irq_restore+0x1a4/0x280 [11480.020053] Call Trace: [11480.020054] [c003bb00] [c003a358] __replay_soft_interrupts+0xe4/0x22c (unreliable) [11480.020060] [c003bcb0] [c003a644] arch_local_irq_restore+0x1a4/0x280 [11480.020064] [c003bcf0] [c02a9d60] tmigr_handle_remote_cpu+0x24c/0x318 [11480.020071] [c003bda0] [c02aa034] tmigr_handle_remote_up+0x208/0x2d0 [11480.020075] [c003be10] [c02a7d34] __walk_groups.isra.0+0x6c/0x100 [11480.020079] [c003be50] [c02aa2d0] tmigr_handle_remote+0xf0/0x170 [11480.020083] [c003bed0] [c02876a4] run_timer_softirq+0x54/0x68 [11480.020089] [c003bef0] [c0179128] handle_softirqs+0x148/0x3b4 [11480.020094] [c003bfe0] [c0017f30] do_softirq_own_stack+0x3c/0x50 [11480.020100] [c2c87900] [c0178688] __irq_exit_rcu+0x18c/0x1b4 [11480.020102] [c2c87930] [c0179758] irq_exit+0x20/0x38 [11480.020105] [c2c87950] [c002b004] timer_interrupt+0x128/0x300 [11480.020108] [c2c879b0] [c0009ffc] decrementer_common_virt+0x28c/0x290 [11480.020113] --- interrupt: 900 at plpar_hcall_norets_notrace+0x18/0x2c [11480.020119] NIP: c00fb9d4 LR: c10c2348 CTR: [11480.020120] REGS: c2c879e0 TRAP: 0900 Tainted: G OE (6.14.0-rc1-next-20250207) [11480.020122] MSR: 8280b033 CR: 22000248 XER: 2004 [11480.020129] CFAR: IRQMASK: 0 [11480.020129] GPR00: c2c87c80 c1667500 [11480.020129] GPR04: [11480.020129] GPR08: 8000c7a3fc00 [11480.020129] GPR12: c300 [11480.020129] GPR16: [11480.020129] GPR20: 00c0 0008 [11480.020129] GPR24: 0a6adcf558a4 [11480.020129] GPR28: 0001 c22618e0 c22618e8 [11480.0
Re: [PATCH v3 2/4] powerpc/fadump: fix additional param memory reservation for HASH MMU
Hi Sourabh, On 23/01/25 5:12 pm, Sourabh Jain wrote: Commit 683eab94da75bc ("powerpc/fadump: setup additional parameters for dump capture kernel") introduced the additional parameter feature in fadump for HASH MMU with the understanding that GRUB does not use the memory area between 640MB and 768MB for its operation. However, the third patch in this series ("powerpc: increase MIN RMA size for CAS negotiation") changes the MIN RMA size to 768MB, allowing GRUB to use memory up to 768MB. This makes the fadump reservation for the additional parameter feature for HASH MMU unreliable. To address this, adjust the memory range for the additional parameter in fadump for HASH MMU. This will ensure that GRUB does not overwrite the memory reserved for fadump's additional parameter in HASH MMU. The new policy for the memory range for the additional parameter in HASH MMU is that the first memory block must be larger than the MIN_RMA size, as the bootloader can use memory up to the MIN_RMA size. The range should be between MIN_RMA and the RMA size (ppc64_rma_size), and it must not overlap with the fadump reserved area. Cc: Avnish Chouhan Cc: Brian King Cc: Hari Bathini Cc: Madhavan Srinivasan Cc: Michael Ellerman Reviewed-by: Mahesh Salgaonkar Signed-off-by: Sourabh Jain --- arch/powerpc/kernel/fadump.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 4b371c738213..26e3d151e048 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -33,6 +33,7 @@ #include #include #include +#include /* * The CPU who acquired the lock to trigger the fadump crash should @@ -1764,19 +1765,19 @@ void __init fadump_setup_param_area(void) range_end = memblock_end_of_DRAM(); } else { /* -* Passing additional parameters is supported for hash MMU only -* if the first memory block size is 768MB or higher. +* Memory range for passing additional parameters for HASH MMU +* must meet the following conditions: +* 1. The first memory block size must be higher than the +*minimum RMA (MIN_RMA) size. Bootloader can use memory +*upto RMA size. So it should be avoided. I believe you mean "upto MIN_RMA size." here? Rest looks good. Reviewed-by: Hari Bathini +* 2. The range should be between MIN_RMA and RMA size (ppc64_rma_size) +* 3. It must not overlap with the fadump reserved area. */ - if (ppc64_rma_size < 0x3000) + if (ppc64_rma_size < MIN_RMA*1024*1024) return; - /* -* 640 MB to 768 MB is not used by PFW/bootloader. So, try reserving -* memory for passing additional parameters in this range to avoid -* being stomped on by PFW/bootloader. -*/ - range_start = 0x2A00; - range_end = range_start + 0x400; + range_start = MIN_RMA * 1024 * 1024; + range_end = min(ppc64_rma_size, fw_dump.boot_mem_top); } fw_dump.param_area = memblock_phys_alloc_range(COMMAND_LINE_SIZE,
Re: [RESEND PATCH] fadump: Use str_yes_no() helper in fadump_show_config()
Hi, Le 09/02/2025 à 09:17, Thorsten Blum a écrit : Remove hard-coded strings by using the str_yes_no() helper function. Reviewed-by: Sourabh Jain Reviewed-by: Ritesh Harjani (IBM) Signed-off-by: Thorsten Blum Any reason for resending ? Your patch is not lost, see https://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=89400 If there is a reason that you don't want to put in the commit message, just put it here below under the --- --- arch/powerpc/kernel/fadump.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 4b371c738213..8c531533dd3e 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -289,10 +289,8 @@ static void __init fadump_show_config(void) if (!fw_dump.fadump_supported) return; - pr_debug("Fadump enabled: %s\n", - (fw_dump.fadump_enabled ? "yes" : "no")); - pr_debug("Dump Active : %s\n", - (fw_dump.dump_active ? "yes" : "no")); + pr_debug("Fadump enabled: %s\n", str_yes_no(fw_dump.fadump_enabled)); + pr_debug("Dump Active : %s\n", str_yes_no(fw_dump.dump_active)); pr_debug("Dump section sizes:\n"); pr_debug("CPU state data size: %lx\n", fw_dump.cpu_state_data_size); pr_debug("HPTE region size : %lx\n", fw_dump.hpte_region_size);
Re: [PATCH net-next] net: phy: remove unused PHY_INIT_TIMEOUT definitions
Le 08/02/2025 à 22:14, Heiner Kallweit a écrit : Both identical definitions of PHY_INIT_TIMEOUT aren't used, so remove them. Would be good to say when it stopped being used, ie which commit or commits removed its use. Also why only remove PHY_INIT_TIMEOUT ? For instance PHY_FORCE_TIMEOUT also seems to be unused. PHY_CHANGE_TIME as well. Signed-off-by: Heiner Kallweit --- drivers/net/ethernet/freescale/ucc_geth.h | 1 - include/linux/phy.h | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h index 38789faae..03b515240 100644 --- a/drivers/net/ethernet/freescale/ucc_geth.h +++ b/drivers/net/ethernet/freescale/ucc_geth.h @@ -890,7 +890,6 @@ struct ucc_geth_hardware_statistics { addresses */ #define TX_TIMEOUT (1*HZ) -#define PHY_INIT_TIMEOUT10 #define PHY_CHANGE_TIME 2 /* Fast Ethernet (10/100 Mbps) */ diff --git a/include/linux/phy.h b/include/linux/phy.h index 3028f8abf..9cb8c 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -293,7 +293,6 @@ static inline long rgmii_clock(int speed) } } -#define PHY_INIT_TIMEOUT 10 #define PHY_FORCE_TIMEOUT 10 #define PHY_MAX_ADDR 32
Re: [RESEND PATCH] fadump: Use str_yes_no() helper in fadump_show_config()
On 9. Feb 2025, at 10:16, Christophe Leroy wrote: > Le 09/02/2025 à 09:17, Thorsten Blum a écrit : >> Remove hard-coded strings by using the str_yes_no() helper function. >> Reviewed-by: Sourabh Jain >> Reviewed-by: Ritesh Harjani (IBM) >> Signed-off-by: Thorsten Blum > > Any reason for resending ? Your patch is not lost, see > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=89400 Ah sorry, I missed this. It's sometimes hard to know the status of a patch when it's not in next yet. Thanks, Thorsten
Re: [PATCH v6 21/26] fs/dax: Properly refcount fs dax pages
On Thu, Feb 06, 2025 at 09:50:07PM -0800, Dan Williams wrote: > Alistair Popple wrote: > > On Mon, Jan 13, 2025 at 07:35:07PM -0800, Dan Williams wrote: > > > Alistair Popple wrote: > > > > [...] > > > > > ...and here is that aformentioned patch: > > > > This patch is different from what you originally posted here: > > https://yhbt.net/lore/linux-s390/172721874675.497781.3277495908107141898.st...@dwillia2-xfh.jf.intel.com/ > > > > > -- 8< -- > > > Subject: dcssblk: Mark DAX broken, remove FS_DAX_LIMITED support > > > > > > From: Dan Williams > > > > > > The dcssblk driver has long needed special case supoprt to enable > > > limited dax operation, so called CONFIG_FS_DAX_LIMITED. This mode > > > works around the incomplete support for ZONE_DEVICE on s390 by forgoing > > > the ability of dax-mapped pages to support GUP. > > > > > > Now, pending cleanups to fsdax that fix its reference counting [1] depend > > > on > > > the ability of all dax drivers to supply ZONE_DEVICE pages. > > > > > > To allow that work to move forward, dax support needs to be paused for > > > dcssblk until ZONE_DEVICE support arrives. That work has been known for > > > a few years [2], and the removal of "pte_devmap" requirements [3] makes > > > the > > > conversion easier. > > > > > > For now, place the support behind CONFIG_BROKEN, and remove PFN_SPECIAL > > > (dcssblk was the only user). > > > > Specifically it no longer removes PFN_SPECIAL. Was this intentional? Or > > should I > > really have picked up the original patch from the mailing list? > > I think this patch that only removes the dccsblk usage of PFN_SPECIAL is > sufficient. Leave the rest to the pfn_t cleanup. Makes sense. I noticed it when rebaing the pfn_t cleanup because previously it did remove PFN_SPECIAL so was just wondering if it was intentional. I will add a patch removing PFN_SPECIAL to the pfn_t/pXX_devmap cleanup series I'm writing now.
Re: [PATCH 3/3] bitmap: break kunit into test cases
On 2/8/25 1:14 AM, Tamir Duberstein wrote: > Move some tests into `bitmap_test_cases` and parameterize > `test_bitmap_print_buf`. This gives us nicer output in the event of a > failure. > > Signed-off-by: Tamir Duberstein Reviewed-by: Muhammad Usama Anjum > --- > lib/bitmap_kunit.c | 182 > ++--- > 1 file changed, 89 insertions(+), 93 deletions(-) > > diff --git a/lib/bitmap_kunit.c b/lib/bitmap_kunit.c > index 0605228288d6..f7b90f6d5f49 100644 > --- a/lib/bitmap_kunit.c > +++ b/lib/bitmap_kunit.c > @@ -17,8 +17,6 @@ > static char pbl_buffer[PAGE_SIZE]; > static char print_buf[PAGE_SIZE * 2]; > > -static struct kunit *kunittest; > - > #define tc_err(fmt, ...) \ > KUNIT_FAIL(kunittest, fmt, ##__VA_ARGS__) > > @@ -96,7 +94,7 @@ static const unsigned long exp3_1_0[] = { > > #define expect_eq_uint(x, y) expect_eq_ulong((unsigned int)(x), > (unsigned int)(y)) > > -static void test_zero_clear(void) > +static void test_zero_clear(struct kunit *kunittest) > { > DECLARE_BITMAP(bmap, 1024); > > @@ -125,7 +123,7 @@ static void test_zero_clear(void) > expect_eq_pbl("", bmap, 1024); > } > > -static void test_find_nth_bit(void) > +static void test_find_nth_bit(struct kunit *kunittest) > { > unsigned long b, bit, cnt = 0; > DECLARE_BITMAP(bmap, 64 * 3); > @@ -166,7 +164,7 @@ static void test_find_nth_bit(void) > } > } > > -static void test_fill_set(void) > +static void test_fill_set(struct kunit *kunittest) > { > DECLARE_BITMAP(bmap, 1024); > > @@ -195,7 +193,7 @@ static void test_fill_set(void) > expect_eq_pbl("0-1023", bmap, 1024); > } > > -static void test_copy(void) > +static void test_copy(struct kunit *kunittest) > { > DECLARE_BITMAP(bmap1, 1024); > DECLARE_BITMAP(bmap2, 1024); > @@ -234,7 +232,7 @@ static void test_copy(void) > expect_eq_pbl("0-108,128-1023", bmap2, 1024); > } > > -static void test_bitmap_region(void) > +static void test_bitmap_region(struct kunit *kunittest) > { > int pos, order; > > @@ -259,7 +257,7 @@ static void test_bitmap_region(void) > > #define EXP2_IN_BITS (sizeof(exp2) * 8) > > -static void test_replace(void) > +static void test_replace(struct kunit *kunittest) > { > unsigned int nbits = 64; > unsigned int nlongs = DIV_ROUND_UP(nbits, BITS_PER_LONG); > @@ -300,7 +298,7 @@ static const unsigned long sg_scatter_exp[] = { > BITMAP_FROM_U64(0x021aULL), > }; > > -static void test_bitmap_sg(void) > +static void test_bitmap_sg(struct kunit *kunittest) > { > unsigned int nbits = 64; > DECLARE_BITMAP(bmap_gather, 100); > @@ -421,7 +419,7 @@ static const struct test_bitmap_parselist > parselist_tests[] = { > > }; > > -static void test_bitmap_parselist(void) > +static void test_bitmap_parselist(struct kunit *kunittest) > { > int i; > int err; > @@ -457,7 +455,7 @@ static void test_bitmap_parselist(void) > } > } > > -static void test_bitmap_printlist(void) > +static void test_bitmap_printlist(struct kunit *kunittest) > { > unsigned long *bmap = kmalloc(PAGE_SIZE, GFP_KERNEL); > char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL); > @@ -533,7 +531,7 @@ static const struct test_bitmap_parselist parse_tests[] = > { > #undef step > }; > > -static void test_bitmap_parse(void) > +static void test_bitmap_parse(struct kunit *kunittest) > { > int i; > int err; > @@ -568,7 +566,7 @@ static void test_bitmap_parse(void) > } > } > > -static void test_bitmap_arr32(void) > +static void test_bitmap_arr32(struct kunit *kunittest) > { > unsigned int nbits, next_bit; > u32 arr[EXP1_IN_BITS / 32]; > @@ -594,7 +592,7 @@ static void test_bitmap_arr32(void) > } > } > > -static void test_bitmap_arr64(void) > +static void test_bitmap_arr64(struct kunit *kunittest) > { > unsigned int nbits, next_bit; > u64 arr[EXP1_IN_BITS / 64]; > @@ -626,7 +624,7 @@ static void test_bitmap_arr64(void) > } > } > > -static noinline void test_mem_optimisations(void) > +static noinline void test_mem_optimisations(struct kunit *kunittest) > { > DECLARE_BITMAP(bmap1, 1024); > DECLARE_BITMAP(bmap2, 1024); > @@ -669,7 +667,7 @@ static const unsigned char clump_exp[] = { > 0x05, /* non-adjacent 2 bits set */ > }; > > -static void test_for_each_set_clump8(void) > +static void test_for_each_set_clump8(struct kunit *kunittest) > { > #define CLUMP_EXP_NUMBITS 64 > DECLARE_BITMAP(bits, CLUMP_EXP_NUMBITS); > @@ -691,7 +689,7 @@ static void test_for_each_set_clump8(void) > expect_eq_clump8(start, CLUMP_EXP_NUMBITS, clump_exp, &clump); > } > > -static void test_for_each_set_bit_wrap(void) > +static void test_for_each_set_bit_wrap(struct kunit *kunittest) > { > DECLARE_BITMAP(orig, 500); > DECLARE_BITMAP(copy, 500); > @@ -716,7 +714,7 @@ static void test_for_each_set_bit_wrap(
Re: [PATCH 2/3] bitmap: convert self-test to KUnit
On 2/8/25 1:14 AM, Tamir Duberstein wrote: > Convert the bitmap() self-test to a KUnit test. > > In the interest of keeping the patch reasonably-sized this doesn't > refactor the tests into proper parameterized tests - it's all one big > test case. > > Signed-off-by: Tamir Duberstein Thanks for the conversion. Reviewed-by: Muhammad Usama Anjum > --- > MAINTAINERS | 2 +- > arch/m68k/configs/amiga_defconfig | 1 - > arch/m68k/configs/apollo_defconfig| 1 - > arch/m68k/configs/atari_defconfig | 1 - > arch/m68k/configs/bvme6000_defconfig | 1 - > arch/m68k/configs/hp300_defconfig | 1 - > arch/m68k/configs/mac_defconfig | 1 - > arch/m68k/configs/multi_defconfig | 1 - > arch/m68k/configs/mvme147_defconfig | 1 - > arch/m68k/configs/mvme16x_defconfig | 1 - > arch/m68k/configs/q40_defconfig | 1 - > arch/m68k/configs/sun3_defconfig | 1 - > arch/m68k/configs/sun3x_defconfig | 1 - > arch/powerpc/configs/ppc64_defconfig | 1 - > lib/Kconfig.debug | 24 ++- > lib/Makefile | 2 +- > lib/{test_bitmap.c => bitmap_kunit.c} | 322 > +- > tools/testing/selftests/lib/bitmap.sh | 3 - > tools/testing/selftests/lib/config| 1 - > 19 files changed, 145 insertions(+), 222 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 896a307fa065..9824d4053748 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4016,11 +4016,11 @@ F:include/linux/nodemask_types.h > F: include/vdso/bits.h > F: lib/bitmap-str.c > F: lib/bitmap.c > +F: lib/bitmap_kunit.c > F: lib/cpumask.c > F: lib/cpumask_kunit.c > F: lib/find_bit.c > F: lib/find_bit_benchmark.c > -F: lib/test_bitmap.c > F: tools/include/linux/bitfield.h > F: tools/include/linux/bitmap.h > F: tools/include/linux/bits.h > diff --git a/arch/m68k/configs/amiga_defconfig > b/arch/m68k/configs/amiga_defconfig > index dbf2ea561c85..3c9d7b58cb8a 100644 > --- a/arch/m68k/configs/amiga_defconfig > +++ b/arch/m68k/configs/amiga_defconfig > @@ -624,7 +624,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/apollo_defconfig > b/arch/m68k/configs/apollo_defconfig > index b0fd199cc0a4..b94c87a7cbdb 100644 > --- a/arch/m68k/configs/apollo_defconfig > +++ b/arch/m68k/configs/apollo_defconfig > @@ -581,7 +581,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/atari_defconfig > b/arch/m68k/configs/atari_defconfig > index bb5b2d3b6c10..823ba96c4486 100644 > --- a/arch/m68k/configs/atari_defconfig > +++ b/arch/m68k/configs/atari_defconfig > @@ -601,7 +601,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/bvme6000_defconfig > b/arch/m68k/configs/bvme6000_defconfig > index 8315a13bab73..0fa985129200 100644 > --- a/arch/m68k/configs/bvme6000_defconfig > +++ b/arch/m68k/configs/bvme6000_defconfig > @@ -573,7 +573,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/hp300_defconfig > b/arch/m68k/configs/hp300_defconfig > index 350370657e5f..75ed6eb547c6 100644 > --- a/arch/m68k/configs/hp300_defconfig > +++ b/arch/m68k/configs/hp300_defconfig > @@ -583,7 +583,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/mac_defconfig b/arch/m68k/configs/mac_defconfig > index f942b4755702..149c398f80a8 100644 > --- a/arch/m68k/configs/mac_defconfig > +++ b/arch/m68k/configs/mac_defconfig > @@ -600,7 +600,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/multi_defconfig > b/arch/m68k/configs/multi_defconfig > index b1eaad02efab..34e6eaa47a18 100644 > --- a/arch/m68k/configs/multi_defconfig > +++ b/arch/m68k/configs/multi_defconfig > @@ -687,7 +687,6 @@ CONFIG_TEST_HEXDUMP=m > CONFIG_TEST_KSTRTOX=m > CONFIG_TEST_PRINTF=m > CONFIG_TEST_SCANF=m > -CONFIG_TEST_BITMAP=m > CONFIG_TEST_UUID=m > CONFIG_TEST_XARRAY=m > CONFIG_TEST_MAPLE_TREE=m > diff --git a/arch/m68k/configs/mvme147_defconfig > b/arch/m68k/configs/mvme
Re: [PATCH 1/3] bitmap: remove _check_eq_u32_array
On 2/8/25 1:14 AM, Tamir Duberstein wrote: > This has been unused since commit 3aa56885e516 ("bitmap: replace > bitmap_{from,to}_u32array") in 2018. Remove it to avoid the need to port > it to KUnit in this series. > > Signed-off-by: Tamir Duberstein Reviewed-by: Muhammad Usama Anjum > --- > lib/test_bitmap.c | 28 > 1 file changed, 28 deletions(-) > > diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c > index 65a75d58ed9e..c83829ef557f 100644 > --- a/lib/test_bitmap.c > +++ b/lib/test_bitmap.c > @@ -100,34 +100,6 @@ __check_eq_pbl(const char *srcfile, unsigned int line, > return true; > } > > -static bool __init > -__check_eq_u32_array(const char *srcfile, unsigned int line, > - const u32 *exp_arr, unsigned int exp_len, > - const u32 *arr, unsigned int len) __used; > -static bool __init > -__check_eq_u32_array(const char *srcfile, unsigned int line, > - const u32 *exp_arr, unsigned int exp_len, > - const u32 *arr, unsigned int len) > -{ > - if (exp_len != len) { > - pr_warn("[%s:%u] array length differ: expected %u, got %u\n", > - srcfile, line, > - exp_len, len); > - return false; > - } > - > - if (memcmp(exp_arr, arr, len*sizeof(*arr))) { > - pr_warn("[%s:%u] array contents differ\n", srcfile, line); > - print_hex_dump(KERN_WARNING, " exp: ", DUMP_PREFIX_OFFSET, > -32, 4, exp_arr, exp_len*sizeof(*exp_arr), false); > - print_hex_dump(KERN_WARNING, " got: ", DUMP_PREFIX_OFFSET, > -32, 4, arr, len*sizeof(*arr), false); > - return false; > - } > - > - return true; > -} > - > static bool __init __check_eq_clump8(const char *srcfile, unsigned int line, > const unsigned int offset, > const unsigned int size, > -- BR, Muhammad Usama Anjum
Re: [PATCH net-next] net: freescale: ucc_geth: remove unused PHY_INIT_TIMEOUT and PHY_CHANGE_TIME
On 09.02.25 13:27, Heiner Kallweit wrote: Both definitions are unused. Last users have been removed with: 1577ecef7666 ("netdev: Merge UCC and gianfar MDIO bus drivers") 728de4c927a3 ("ucc_geth: migrate ucc_geth to phylib") Signed-off-by: Heiner Kallweit --- drivers/net/ethernet/freescale/ucc_geth.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h index 38789faae..84f92f638 100644 --- a/drivers/net/ethernet/freescale/ucc_geth.h +++ b/drivers/net/ethernet/freescale/ucc_geth.h @@ -890,8 +890,6 @@ struct ucc_geth_hardware_statistics { addresses */ #define TX_TIMEOUT (1*HZ) -#define PHY_INIT_TIMEOUT10 -#define PHY_CHANGE_TIME 2 /* Fast Ethernet (10/100 Mbps) */ #define UCC_GETH_URFS_INIT 512 /* Rx virtual FIFO size Reviewed-by: Gerhard Engleder
Re: [PATCH 2/2] powerpc/fadump: fix additional param memory reservation for HASH MMU
On 04/02/25 2:07 pm, Avnish Chouhan wrote: On 2025-02-04 11:57, Hari Bathini wrote: On 04/02/25 10:58 am, Avnish Chouhan wrote: On 2025-01-31 20:44, Hari Bathini wrote: On 23/01/25 7:54 pm, Avnish Chouhan wrote: On 2025-01-23 15:26, Hari Bathini wrote: On 20/01/25 11:05 pm, Sourabh Jain wrote: Commit 683eab94da75bc ("powerpc/fadump: setup additional parameters for dump capture kernel") introduced the additional parameter feature in fadump for HASH MMU with the understanding that GRUB does not use the memory area between 640MB and 768MB for its operation. However, the patch ("powerpc: increase MIN RMA size for CAS negotiation") changes the MIN RMA size to 768MB, allowing GRUB to use memory up to 768MB. This makes the fadump reservation for the additional parameter feature for HASH MMU unreliable. To address this, adjust the memory range for the additional parameter in fadump for HASH MMU. This will ensure that GRUB does not overwrite the memory reserved for fadump's additional parameter in HASH MMU. The new policy for the memory range for the additional parameter in HASH MMU is that the first memory block must be larger than the MIN_RMA size, as the bootloader can use memory up to the MIN_RMA size. The range should be between MIN_RMA and the RMA size (ppc64_rma_size), and it must not overlap with the fadump reserved area. IIRC, even memory above MIN_RMA is used by the bootloader except for 640MB to 768MB (assuming RMA size is >768MB). So, how does this change guarantee that the bootloader is not using memory reserved for bootargs? Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) starting top-down at 768MB earlier. With MIN_RMA changed to 768MB, is bootloader still using the concept of RUNTIME_MIN_SPACE to set aside some memory for kernel to use. If yes, where exactly is it allocating this space now? Also, rtas instantiates top-down at 768MB. Would that not have a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB? - Hari Hi Hari, Hi Avnish, The RUNTIME_MIN_SPACE is the space left aside by Grub is within the MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub will still keep the 128 MB space in MIN_RMA for loading stock kernel and initrd. IIUC, you mean, 640MB to 768MB is not used by Grub even if MIN_RMA is at 768MB? If that is true, this change is not needed, as fadump could still use the memory between 640MB to 768MB, right? Am I missing something here.. Hari, No. As we are changing MIN_RMA to 768 MB, GRUB can use memory till 768 MB if required. Does that mean 'linux_rmo_save' related code in grub-core/kern/ieee1275/init.c is going to be dead code after this change. Also, does this imply, there isn't going to be any RUNTIME_MIN_SPACE support for linux in grub? No Hari, there's no change in RUNTIME_MIN_SPACE as mentioned earlier nor the change leading to any dead code in grub. If we have MIN_RMA as 512 MB, the grub will consider RUNTIME_MIN_SPACE region within the MIN_RMA as (384[512-128] to 512). And if we have MIN_RMA as 768 MB, it will be (640[768-128] to 768). Grub will keep the 128 MB space in MIN_RMA for loading stock kernel and initrd as stated earlier. Thanks, Avnish. That clears it. - Hari
Re: [PATCH 1/9] of: Add warpper function of_find_node_by_name_balanced()
Hi, Laurent, I think we all agree that of_find_node_by_name() is miused, and that it shows the API isn't optimal. What we have different opinions on is how to make the API less error-prone. I think adding a new of_find_node_by_name_balanced() function works around the issue and doesn't improve the situation much, I would argue it makes things even more confusing. We have only 20 calls to of_find_node_by_name() with a non-NULL first argument in v6.14-rc1: arch/powerpc/platforms/chrp/pci.c: rtas = of_find_node_by_name (root, "rtas"); The 'root' variable here is the result of a call to 'of_find_node_by_path("/")', so I think we could pass a null pointer instead to simplify things. arch/powerpc/platforms/powermac/pic.c: slave = of_find_node_by_name(master, "mac-io"); Here I believe of_find_node_by_name() is called to find a *child* node of 'master'. of_find_node_by_name() is the wrong function for that. arch/sparc/kernel/leon_kernel.c:np = of_find_node_by_name(rootnp, "GAISLER_IRQMP"); arch/sparc/kernel/leon_kernel.c:np = of_find_node_by_name(rootnp, "01_00d"); arch/sparc/kernel/leon_kernel.c:np = of_find_node_by_name(nnp, "GAISLER_GPTIMER"); arch/sparc/kernel/leon_kernel.c:np = of_find_node_by_name(nnp, "01_011"); Here too the code seems to be looking for child nodes only (but I couldn't find a DT example or binding in-tree, so I'm not entirely sure). drivers/clk/ti/clk.c: return of_find_node_by_name(from, tmp); Usage here seems correct, the reference-count decrement is intended. drivers/media/i2c/max9286.c:i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux"); drivers/media/platform/qcom/venus/core.c: enp = of_find_node_by_name(dev->of_node, node_name); drivers/net/dsa/bcm_sf2.c: ports = of_find_node_by_name(dn, "ports"); drivers/net/dsa/hirschmann/hellcreek_ptp.c: leds = of_find_node_by_name(hellcreek->dev->of_node, "leds"); drivers/net/ethernet/broadcom/asp2/bcmasp.c:ports_node = of_find_node_by_name(dev->of_node, "ethernet-ports"); drivers/net/ethernet/marvell/prestera/prestera_main.c: ports = of_find_node_by_name(sw->np, "ports"); drivers/net/pse-pd/tps23881.c: channels_node = of_find_node_by_name(priv->np, "channels"); drivers/regulator/scmi-regulator.c: np = of_find_node_by_name(handle->dev->of_node, "regulators"); drivers/regulator/tps6594-regulator.c: np = of_find_node_by_name(tps->dev->of_node, multi_regs[multi].supply_name); Incorrect usage, as far as I understand all those drivers are looking for child nodes only. drivers/of/unittest.c: found = of_find_node_by_name(nd->overlay, "test-unittest16"); drivers/of/unittest.c: found = of_find_node_by_name(nd->overlay, "test-unittest17"); drivers/of/unittest.c: found = of_find_node_by_name(nd->overlay, "test-unittest18"); drivers/of/unittest.c: found = of_find_node_by_name(nd->overlay, "test-unittest19"); Here too I think only child nodes are meant to be considered. of_find_node_by_name() is very much misused as most callers want to find child nodes, while of_find_node_by_name() will walk the whole DT from a given starting point. I think the right fix here is to - Replace of_find_node_by_name(root, ...) with of_find_node_by_name(NULL, ...) in arch/powerpc/platforms/chrp/pci.c (if my understanding of the code is correct). For arch/powerpc/platforms/chrp/pci.c, noticing that there is a comment in setup_peg2(): /* keep the reference to the root node */ It might can not be convert to of_find_node_by_name(NULL, ...), and the origin use of of_find_node_by_name() put the ref count which want to be kept. - Replace of_find_node_by_name() with of_get_child_by_name() in callers that need to search immediate children only (I expected that to be the majority of the above call sites) Since there is no enough information about these DT nodes, it would take time to prove if it is OK to make such convert. - If there are other callers that need to find indirect children, introduce a new of_get_child_by_name_recursive() function. At that point, the only remaining caller of of_find_node_by_name() (beside its usage in the for_each_node_by_name() macro) will be drivers/clk/ti/clk.c, which uses the function correctly. I'm tempted to then rename of_find_node_by_name() to __of_find_node_by_name() to indicate it's an internal function not meant to be called except in special cases. It could all be renamed to __of_find_next_node_by_name() to make its behaviour clearer. The actual code logic of of_find_node_by_name() is more suitable to be used in a loop.So,rename of_find_node_by_name() to __of_find_next_node_by_name() seems to be a good idea. Best regards, Zekun
Re: [PATCH net-next] net: phy: remove unused PHY_INIT_TIMEOUT definitions
On 09.02.2025 10:28, Christophe Leroy wrote: > > > Le 08/02/2025 à 22:14, Heiner Kallweit a écrit : >> Both identical definitions of PHY_INIT_TIMEOUT aren't used, >> so remove them. > > Would be good to say when it stopped being used, ie which commit or commits > removed its use. > > Also why only remove PHY_INIT_TIMEOUT ? For instance PHY_FORCE_TIMEOUT also > seems to be unused. PHY_CHANGE_TIME as well. > I stumbled just across PHY_INIT_TIMEOUT. You're right, I will include other apparently unused definitions as well. >> >> Signed-off-by: Heiner Kallweit >> --- >> drivers/net/ethernet/freescale/ucc_geth.h | 1 - >> include/linux/phy.h | 1 - >> 2 files changed, 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/ucc_geth.h >> b/drivers/net/ethernet/freescale/ucc_geth.h >> index 38789faae..03b515240 100644 >> --- a/drivers/net/ethernet/freescale/ucc_geth.h >> +++ b/drivers/net/ethernet/freescale/ucc_geth.h >> @@ -890,7 +890,6 @@ struct ucc_geth_hardware_statistics { >> addresses */ >> #define TX_TIMEOUT (1*HZ) >> -#define PHY_INIT_TIMEOUT 10 >> #define PHY_CHANGE_TIME 2 >> /* Fast Ethernet (10/100 Mbps) */ >> diff --git a/include/linux/phy.h b/include/linux/phy.h >> index 3028f8abf..9cb8c 100644 >> --- a/include/linux/phy.h >> +++ b/include/linux/phy.h >> @@ -293,7 +293,6 @@ static inline long rgmii_clock(int speed) >> } >> } >> -#define PHY_INIT_TIMEOUT 10 >> #define PHY_FORCE_TIMEOUT 10 >> #define PHY_MAX_ADDR 32 > -- pw-bot: cr
Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().
On 2/8/25 23:25, Christophe Leroy wrote: Le 08/02/2025 à 14:42, Shrikanth Hegde a écrit : On 2/8/25 18:25, Christophe Leroy wrote: Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit : On 2/4/25 13:52, Sebastian Andrzej Siewior wrote: Use preempt_model_str() instead of manually conducting the preemption model. Use pr_emerg() instead of printk() to pass a loglevel. even on powerpc, i see __die ends up calling show_regs_print_info(). Why print it twice? I don't understand what you mean, what is printed twice ? I can't see show_regs_print_info() printing the preemption model, am I missing something ? Patch 2/9 add preemption string in dump_stack_print_info. __die -> show_regs() _> show_regs_print_info() -> dump_stack_print_info() -> init_utsname()->version, preempt_model_str(), BUILD_ID_VAL); Wont we end up in this path? Indeed I missed that. You are right, we now get the information twice: I think we can remove it from arch specific code and rely on lib/dump_stack? And similar concern of printk vs pr_warn/pr_emerg would apply to that as well i guess. [ 440.068216] BUG: Unable to handle kernel data access on write at 0xc09036fc
[PATCH net-next] net: freescale: ucc_geth: remove unused PHY_INIT_TIMEOUT and PHY_CHANGE_TIME
Both definitions are unused. Last users have been removed with: 1577ecef7666 ("netdev: Merge UCC and gianfar MDIO bus drivers") 728de4c927a3 ("ucc_geth: migrate ucc_geth to phylib") Signed-off-by: Heiner Kallweit --- drivers/net/ethernet/freescale/ucc_geth.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h index 38789faae..84f92f638 100644 --- a/drivers/net/ethernet/freescale/ucc_geth.h +++ b/drivers/net/ethernet/freescale/ucc_geth.h @@ -890,8 +890,6 @@ struct ucc_geth_hardware_statistics { addresses */ #define TX_TIMEOUT (1*HZ) -#define PHY_INIT_TIMEOUT10 -#define PHY_CHANGE_TIME 2 /* Fast Ethernet (10/100 Mbps) */ #define UCC_GETH_URFS_INIT 512/* Rx virtual FIFO size -- 2.48.1
[RESEND PATCH] fadump: Use str_yes_no() helper in fadump_show_config()
Remove hard-coded strings by using the str_yes_no() helper function. Reviewed-by: Sourabh Jain Reviewed-by: Ritesh Harjani (IBM) Signed-off-by: Thorsten Blum --- arch/powerpc/kernel/fadump.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index 4b371c738213..8c531533dd3e 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -289,10 +289,8 @@ static void __init fadump_show_config(void) if (!fw_dump.fadump_supported) return; - pr_debug("Fadump enabled: %s\n", - (fw_dump.fadump_enabled ? "yes" : "no")); - pr_debug("Dump Active : %s\n", - (fw_dump.dump_active ? "yes" : "no")); + pr_debug("Fadump enabled: %s\n", str_yes_no(fw_dump.fadump_enabled)); + pr_debug("Dump Active : %s\n", str_yes_no(fw_dump.dump_active)); pr_debug("Dump section sizes:\n"); pr_debug("CPU state data size: %lx\n", fw_dump.cpu_state_data_size); pr_debug("HPTE region size : %lx\n", fw_dump.hpte_region_size); -- 2.48.1
Re: [PATCH v2 09/12] dt-bindings: memory-controllers: Convert fsl,elbc to YAML
On Sun, Feb 09, 2025 at 06:30:44PM +0100, Krzysztof Kozlowski wrote: > On 09/02/2025 18:28, J. Neuschäfer wrote: > > On Fri, Feb 07, 2025 at 05:44:59PM -0600, Rob Herring (Arm) wrote: > >> On Fri, 07 Feb 2025 22:30:26 +0100, J. Neuschäfer wrote: > > [...] > >>> .../bindings/memory-controllers/fsl,elbc.yaml | 146 > >>> + > >>> .../devicetree/bindings/powerpc/fsl/lbc.txt| 43 -- > >>> 2 files changed, 146 insertions(+), 43 deletions(-) > > [...] > >> dtschema/dtc warnings/errors: > >> Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: > >> /example-0/localbus@f0010100/simple-periph@2,0: failed to match any schema > >> with compatible: ['fsl,elbc-gpcm-uio'] > >> Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: > >> /example-1/localbus@e0005000/nand@1,0: failed to match any schema with > >> compatible: ['fsl,mpc8315-fcm-nand', 'fsl,elbc-fcm-nand'] > >> Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: > >> /example-1/localbus@e0005000/nand@1,0: failed to match any schema with > >> compatible: ['fsl,mpc8315-fcm-nand', 'fsl,elbc-fcm-nand'] > > > > I think this is due to how the patches are ordered in the series. > > If that's possible, this should be fixed, e.g. by re-ordering the patches. Yes, I'll do that for the next iteration Best regards, J. Neuschäfer
Re: [PATCH v2 09/12] dt-bindings: memory-controllers: Convert fsl,elbc to YAML
On Fri, Feb 07, 2025 at 05:44:59PM -0600, Rob Herring (Arm) wrote: > On Fri, 07 Feb 2025 22:30:26 +0100, J. Neuschäfer wrote: [...] > > .../bindings/memory-controllers/fsl,elbc.yaml | 146 > > + > > .../devicetree/bindings/powerpc/fsl/lbc.txt| 43 -- > > 2 files changed, 146 insertions(+), 43 deletions(-) [...] > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: > /example-0/localbus@f0010100/simple-periph@2,0: failed to match any schema > with compatible: ['fsl,elbc-gpcm-uio'] > Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: > /example-1/localbus@e0005000/nand@1,0: failed to match any schema with > compatible: ['fsl,mpc8315-fcm-nand', 'fsl,elbc-fcm-nand'] > Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: > /example-1/localbus@e0005000/nand@1,0: failed to match any schema with > compatible: ['fsl,mpc8315-fcm-nand', 'fsl,elbc-fcm-nand'] I think this is due to how the patches are ordered in the series. This patch uses fsl,elbc-gpcm-uio and fsl,elbc-fcm-nand in examples, but comes before the patches that define the corresponding bindings.
Re: [PATCH v2 09/12] dt-bindings: memory-controllers: Convert fsl,elbc to YAML
On 09/02/2025 18:28, J. Neuschäfer wrote: > On Fri, Feb 07, 2025 at 05:44:59PM -0600, Rob Herring (Arm) wrote: >> On Fri, 07 Feb 2025 22:30:26 +0100, J. Neuschäfer wrote: > [...] >>> .../bindings/memory-controllers/fsl,elbc.yaml | 146 >>> + >>> .../devicetree/bindings/powerpc/fsl/lbc.txt| 43 -- >>> 2 files changed, 146 insertions(+), 43 deletions(-) > [...] >> dtschema/dtc warnings/errors: >> Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: >> /example-0/localbus@f0010100/simple-periph@2,0: failed to match any schema >> with compatible: ['fsl,elbc-gpcm-uio'] >> Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: >> /example-1/localbus@e0005000/nand@1,0: failed to match any schema with >> compatible: ['fsl,mpc8315-fcm-nand', 'fsl,elbc-fcm-nand'] >> Documentation/devicetree/bindings/memory-controllers/fsl,elbc.example.dtb: >> /example-1/localbus@e0005000/nand@1,0: failed to match any schema with >> compatible: ['fsl,mpc8315-fcm-nand', 'fsl,elbc-fcm-nand'] > > I think this is due to how the patches are ordered in the series. If that's possible, this should be fixed, e.g. by re-ordering the patches. > This patch uses fsl,elbc-gpcm-uio and fsl,elbc-fcm-nand in examples, but > comes before the patches that define the corresponding bindings. Best regards, Krzysztof
Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().
Le 09/02/2025 à 15:38, Shrikanth Hegde a écrit : On 2/8/25 23:25, Christophe Leroy wrote: Le 08/02/2025 à 14:42, Shrikanth Hegde a écrit : On 2/8/25 18:25, Christophe Leroy wrote: Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit : On 2/4/25 13:52, Sebastian Andrzej Siewior wrote: Use preempt_model_str() instead of manually conducting the preemption model. Use pr_emerg() instead of printk() to pass a loglevel. even on powerpc, i see __die ends up calling show_regs_print_info(). Why print it twice? I don't understand what you mean, what is printed twice ? I can't see show_regs_print_info() printing the preemption model, am I missing something ? Patch 2/9 add preemption string in dump_stack_print_info. __die -> show_regs() _> show_regs_print_info() -> dump_stack_print_info() -> init_utsname()->version, preempt_model_str(), BUILD_ID_VAL); Wont we end up in this path? Indeed I missed that. You are right, we now get the information twice: I think we can remove it from arch specific code and rely on lib/ dump_stack? Yes I guess so. And similar concern of printk vs pr_warn/pr_emerg would apply to that as well i guess. Well, powerpc's show_regs() calls it with show_regs_print_info(KERN_DEFAULT); And dump_stack_print_info() uses printk with log_lvl so there should be no concern here. [ 440.068216] BUG: Unable to handle kernel data access on write at 0xc09036fc