Re: [PATCH v5 04/15] sparc: simplify module_alloc()

[Sam Ravnborg]

Hi Mike,
On Mon, Apr 22, 2024 at 12:44:25PM +0300, Mike Rapoport wrote:
> From: "Mike Rapoport (IBM)" 
> 
> Define MODULES_VADDR and MODULES_END as VMALLOC_START and VMALLOC_END
> for 32-bit and reduce module_alloc() to
> 
>   __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, ...)
> 
> as with the new defines the allocations becames identical for both 32
> and 64 bits.
> 
> While on it, drop unsed include of 
> 
> Suggested-by: Sam Ravnborg 
> Signed-off-by: Mike Rapoport (IBM) 

Looks good.
Reviewed-by: Sam Ravnborg 

Re: [PATCH v4 06/15] mm/execmem, arch: convert simple overrides of module_alloc to execmem

[Sam Ravnborg]

Hi Mike.

On Thu, Apr 11, 2024 at 07:00:42PM +0300, Mike Rapoport wrote:
> From: "Mike Rapoport (IBM)" 
> 
> Several architectures override module_alloc() only to define address
> range for code allocations different than VMALLOC address space.
> 
> Provide a generic implementation in execmem that uses the parameters for
> address space ranges, required alignment and page protections provided
> by architectures.
> 
> The architectures must fill execmem_info structure and implement
> execmem_arch_setup() that returns a pointer to that structure. This way the
> execmem initialization won't be called from every architecture, but rather
> from a central place, namely a core_initcall() in execmem.
> 
> The execmem provides execmem_alloc() API that wraps __vmalloc_node_range()
> with the parameters defined by the architectures.  If an architecture does
> not implement execmem_arch_setup(), execmem_alloc() will fall back to
> module_alloc().
> 
> Signed-off-by: Mike Rapoport (IBM) 
> ---

This code snippet could be more readable ...
> diff --git a/arch/sparc/kernel/module.c b/arch/sparc/kernel/module.c
> index 66c45a2764bc..b70047f944cc 100644
> --- a/arch/sparc/kernel/module.c
> +++ b/arch/sparc/kernel/module.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -21,34 +22,26 @@
>  
>  #include "entry.h"
>  
> +static struct execmem_info execmem_info __ro_after_init = {
> + .ranges = {
> + [EXECMEM_DEFAULT] = {
>  #ifdef CONFIG_SPARC64
> -
> -#include 
> -
> -static void *module_map(unsigned long size)
> -{
> - if (PAGE_ALIGN(size) > MODULES_LEN)
> - return NULL;
> - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
> - GFP_KERNEL, PAGE_KERNEL, 0, NUMA_NO_NODE,
> - __builtin_return_address(0));
> -}
> + .start = MODULES_VADDR,
> + .end = MODULES_END,
>  #else
> -static void *module_map(unsigned long size)
> + .start = VMALLOC_START,
> + .end = VMALLOC_END,
> +#endif
> + .alignment = 1,
> + },
> + },
> +};
> +
> +struct execmem_info __init *execmem_arch_setup(void)
>  {
> - return vmalloc(size);
> -}
> -#endif /* CONFIG_SPARC64 */
> -
> -void *module_alloc(unsigned long size)
> -{
> - void *ret;
> -
> - ret = module_map(size);
> - if (ret)
> - memset(ret, 0, size);
> + execmem_info.ranges[EXECMEM_DEFAULT].pgprot = PAGE_KERNEL;
>  
> - return ret;
> + return _info;
>  }
>  
>  /* Make generic code ignore STT_REGISTER dummy undefined symbols.  */

... if the following was added:

diff --git a/arch/sparc/include/asm/pgtable_32.h 
b/arch/sparc/include/asm/pgtable_32.h
index 9e85d57ac3f2..62bcafe38b1f 100644
--- a/arch/sparc/include/asm/pgtable_32.h
+++ b/arch/sparc/include/asm/pgtable_32.h
@@ -432,6 +432,8 @@ static inline int io_remap_pfn_range(struct vm_area_struct 
*vma,

 #define VMALLOC_START   _AC(0xfe60,UL)
 #define VMALLOC_END _AC(0xffc0,UL)
+#define MODULES_VADDR   VMALLOC_START
+#define MODULES_END VMALLOC_END


Then the #ifdef CONFIG_SPARC64 could be dropped and the code would be
the same for 32 and 64 bits.

Just a drive-by comment.

Sam

Re: [PATCH v2 3/3] arch: Rename fbdev header and source files

[Sam Ravnborg]

Hi Thomas,

On Wed, Mar 27, 2024 at 09:41:31PM +0100, Thomas Zimmermann wrote:
> The per-architecture fbdev code has no dependencies on fbdev and can
> be used for any video-related subsystem. Rename the files to 'video'.
> Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE.
> 
> Further update all includes statements, includ guards, and Makefiles.
^ missing 'e' 

> Also update a few strings and comments to refer to video instead of
> fbdev.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Vineet Gupta 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Huacai Chen 
> Cc: WANG Xuerui 
> Cc: Geert Uytterhoeven 
> Cc: Thomas Bogendoerfer 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: Michael Ellerman 
> Cc: Nicholas Piggin 
> Cc: Yoshinori Sato 
> Cc: Rich Felker 
> Cc: John Paul Adrian Glaubitz 
> Cc: "David S. Miller" 
> Cc: Andreas Larsson 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 

If the patch is changed to use the Kbuild file to pick the generic
variant of video.h then it is:

Reviewed-by: Sam Ravnborg 

I also added an unrelated sparc comment, that can be addressed another
time.

Sam

> ---
>  arch/arc/include/asm/fb.h|  8 
>  arch/arc/include/asm/video.h |  8 
>  arch/arm/include/asm/fb.h|  6 --
>  arch/arm/include/asm/video.h |  6 ++
>  arch/arm64/include/asm/fb.h  | 10 --
>  arch/arm64/include/asm/video.h   | 10 ++
>  arch/loongarch/include/asm/{fb.h => video.h} |  8 
>  arch/m68k/include/asm/{fb.h => video.h}  |  8 
>  arch/mips/include/asm/{fb.h => video.h}  | 12 ++--
>  arch/parisc/include/asm/{fb.h => video.h}|  8 
>  arch/parisc/video/Makefile   |  2 +-
>  arch/parisc/video/{fbdev.c => video-sti.c}   |  2 +-
>  arch/powerpc/include/asm/{fb.h => video.h}   |  8 
>  arch/powerpc/kernel/pci-common.c |  2 +-
>  arch/sh/include/asm/fb.h |  7 ---
>  arch/sh/include/asm/video.h  |  7 +++
>  arch/sparc/include/asm/{fb.h => video.h} |  8 
>  arch/sparc/video/Makefile|  2 +-
>  arch/sparc/video/{fbdev.c => video.c}|  4 ++--
>  arch/x86/include/asm/{fb.h => video.h}   |  8 
>  arch/x86/video/Makefile  |  2 +-
>  arch/x86/video/{fbdev.c => video.c}  |  3 ++-
>  include/asm-generic/Kbuild   |  2 +-
>  include/asm-generic/{fb.h => video.h}|  6 +++---
>  include/linux/fb.h   |  2 +-
>  25 files changed, 75 insertions(+), 74 deletions(-)
>  delete mode 100644 arch/arc/include/asm/fb.h
>  create mode 100644 arch/arc/include/asm/video.h
>  delete mode 100644 arch/arm/include/asm/fb.h
>  create mode 100644 arch/arm/include/asm/video.h
>  delete mode 100644 arch/arm64/include/asm/fb.h
>  create mode 100644 arch/arm64/include/asm/video.h
>  rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
>  rename arch/m68k/include/asm/{fb.h => video.h} (86%)
>  rename arch/mips/include/asm/{fb.h => video.h} (76%)
>  rename arch/parisc/include/asm/{fb.h => video.h} (68%)
>  rename arch/parisc/video/{fbdev.c => video-sti.c} (96%)
>  rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
>  delete mode 100644 arch/sh/include/asm/fb.h
>  create mode 100644 arch/sh/include/asm/video.h
>  rename arch/sparc/include/asm/{fb.h => video.h} (89%)
>  rename arch/sparc/video/{fbdev.c => video.c} (86%)
>  rename arch/x86/include/asm/{fb.h => video.h} (77%)
>  rename arch/x86/video/{fbdev.c => video.c} (97%)
>  rename include/asm-generic/{fb.h => video.h} (96%)
> 
> diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h
> deleted file mode 100644
> index 9c2383d29cbb9..0
> --- a/arch/arc/include/asm/fb.h
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -
> -#ifndef _ASM_FB_H_
> -#define _ASM_FB_H_
> -
> -#include 
> -
> -#endif /* _ASM_FB_H_ */
> diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h
> new file mode 100644
> index 0..8ff7263727fe7
> --- /dev/null
> +++ b/arch/arc/include/asm/video.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_VIDEO_H_
> +#define _ASM_VIDEO_H_
> +
> +#include 
> +
> +#endif /* _ASM_VIDEO_H_ */

arch/arc/include/asm/video.h o

Re: [PATCH v2 1/3] arch: Select fbdev helpers with CONFIG_VIDEO

[Sam Ravnborg]

On Wed, Mar 27, 2024 at 09:41:29PM +0100, Thomas Zimmermann wrote:
> Various Kconfig options selected the per-architecture helpers for
> fbdev. But none of the contained code depends on fbdev. Standardize
> on CONFIG_VIDEO, which will allow to add more general helpers for
> video functionality.
> 
> CONFIG_VIDEO protects each architecture's video/ directory. This
> allows for the use of more fine-grained control for each directory's
> files, such as the use of CONFIG_STI_CORE on parisc.
> 
> v2:
> - sparc: rebased onto Makefile changes
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: "David S. Miller" 
> Cc: Andreas Larsson 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 

Reviewed-by: Sam Ravnborg 

Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers

[Sam Ravnborg]

Hi Thomas,
On Wed, Mar 27, 2024 at 09:41:30PM +0100, Thomas Zimmermann wrote:
> The per-architecture video helpers do not depend on struct fb_info
> or anything else from fbdev. Remove it from the interface and replace
> fb_is_primary_device() with video_is_primary_device(). The new helper
> is similar in functionality, but can operate on non-fbdev devices.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: "David S. Miller" 
> Cc: Andreas Larsson 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 
Reviewed-by: Sam Ravnborg 

Re: [PATCH 09/22] [v2] arch: fix asm-offsets.c building with -Wmissing-prototypes

[Sam Ravnborg]

On Wed, Nov 08, 2023 at 01:58:30PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> When -Wmissing-prototypes is enabled, the some asm-offsets.c files fail
> to build, even when this warning is disabled in the Makefile for normal
> files:
> 
> arch/sparc/kernel/asm-offsets.c:22:5: error: no previous prototype for 
> 'sparc32_foo' [-Werror=missing-prototypes]
> arch/sparc/kernel/asm-offsets.c:48:5: error: no previous prototype for 'foo' 
> [-Werror=missing-prototypes]
> 
> Address this by making use of the same trick as x86, marking these
> functions as 'static __used' to avoid the need for a prototype
> by not drop them in dead-code elimination.
> 
> Suggested-by: Masahiro Yamada 
> Link: 
> https://lore.kernel.org/lkml/cak7lnarfemfk0du4hed19ex_g6tuc5wg0zp+l1ayvdpof4y...@mail.gmail.com/
> Signed-off-by: Arnd Bergmann 
Looks good. I sometimes looks at sparc patches so I looked at this one.
Reviewed-by: Sam Ravnborg 

Re: [PATCH 4/9] sparc: Remove

[Sam Ravnborg]

On Thu, Aug 17, 2023 at 06:07:35PM +0200, Geert Uytterhoeven wrote:
> As of commit b7fb14d3ac63117e ("ide: remove the legacy ide driver") in
> v5.14, there are no more generic users of .
> 
> Signed-off-by: Geert Uytterhoeven 
Acked-by: Sam Ravnborg 

Re: [PATCH] fbdev/ps3fb: Build without kernel device

[Sam Ravnborg]

On Mon, Jul 31, 2023 at 07:55:00PM +0200, Thomas Zimmermann wrote:
> Use fb_info() to print status message at the end of the probe function,
> which avoids decoding the devices. fb_info() works with or without an
> fbdev kernel device. Fixes the following error:
> 
> ../drivers/video/fbdev/ps3fb.c: In function 'ps3fb_probe':
> ../drivers/video/fbdev/ps3fb.c:1172:40: error: 'struct fb_info' has no member 
> named 'dev'
>  1172 |  dev_driver_string(info->dev), dev_name(info->dev),
>   |^~
> ../include/linux/dev_printk.h:110:37: note: in definition of macro 
> 'dev_printk_index_wrap'
>   110 | _p_func(dev, fmt, ##__VA_ARGS__); 
>   \
>   | ^~~
> ../drivers/video/fbdev/ps3fb.c:1171:9: note: in expansion of macro 'dev_info'
>  1171 | dev_info(info->device, "%s %s, using %u KiB of video 
> memory\n",
>   | ^~~~
> ../drivers/video/fbdev/ps3fb.c:1172:61: error: 'struct fb_info' has no member 
> named 'dev'
>  1172 |  dev_driver_string(info->dev), dev_name(info->dev),
>   | ^~
> ../include/linux/dev_printk.h:110:37: note: in definition of macro 
> 'dev_printk_index_wrap'
>   110 | _p_func(dev, fmt, ##__VA_ARGS__); 
>   \
>   | ^~~
> ../drivers/video/fbdev/ps3fb.c:1171:9: note: in expansion of macro 'dev_info'
>  1171 | dev_info(info->device, "%s %s, using %u KiB of video 
> memory\n",
>   | ^~~~
> 
> Reported-by: Randy Dunlap 
> Closes: 
> https://lore.kernel.org/lkml/ccc63065-2976-88ef-1211-731330bf2...@infradead.org/
> Signed-off-by: Thomas Zimmermann 
> Fixes: 701d2054fa31 ("fbdev: Make support for userspace interfaces 
> configurable")
> Cc: Michael Ellerman 
> Cc: Sam Ravnborg 
> Cc: Helge Deller 
> Cc: Javier Martinez Canillas 
> Cc: Randy Dunlap 
> Cc: Bagas Sanjaya 
> Cc: Thorsten Leemhuis 
> Cc: dri-de...@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
Acked-by: Sam Ravnborg 
> ---
>  drivers/video/fbdev/ps3fb.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
> index 5aedc30c5f7e..64d291d6b153 100644
> --- a/drivers/video/fbdev/ps3fb.c
> +++ b/drivers/video/fbdev/ps3fb.c
> @@ -1168,9 +1168,7 @@ static int ps3fb_probe(struct ps3_system_bus_device 
> *dev)
>  
>   ps3_system_bus_set_drvdata(dev, info);
>  
> - dev_info(info->device, "%s %s, using %u KiB of video memory\n",
> -  dev_driver_string(info->dev), dev_name(info->dev),
> -  info->fix.smem_len >> 10);
> + fb_info(info, "using %u KiB of video memory\n", info->fix.smem_len >> 
> 10);
>  
>   task = kthread_run(ps3fbd, info, DEVICE_NAME);
>   if (IS_ERR(task)) {
> -- 
> 2.41.0

Re: [PATCH] powerpc: Use shared font data

[Sam Ravnborg]

Hi David,

On Tue, Jul 25, 2023 at 01:01:41AM +0100, li...@treblig.org wrote:
> From: "Dr. David Alan Gilbert" 
> 
> PowerPC has a 'btext' font used for the console which is almost identical
> to the shared font_sun8x16, so use it rather than duplicating the data.
> 
> They were actually identical until about a decade ago when
>commit bcfbeecea11c ("drivers: console: font_: Change a glyph from
> "broken bar" to "vertical line"")
> 
> which changed the | in the shared font to be a solid
> bar rather than a broken bar.  That's the only difference.
> 
> This was originally spotted by PMD which noticed that sparc does
> the same thing with the same data, and they also share a bunch
> of functions to manipulate the data.  I've previously posted a near
> identical patch for sparc.
> 
> One difference I notice in PowerPC is that there are a bunch of compile
> options for the .c files for the early code to avoid a bunch of security
> compilation features;  it's not clear to me if this is a problem for
> this font data.
> 
> Tested very lightly with a boot without FS in qemu.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Yep, looks very similar to sparc, so

Reviewed-by: Sam Ravnborg 

Re: [PATCH 00/17] fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags

[Sam Ravnborg]

Hi Thomas,

On Tue, Jul 11, 2023 at 08:24:40AM +0200, Thomas Zimmermann wrote:
> Hi Sam
> 
> Am 10.07.23 um 19:19 schrieb Sam Ravnborg:
> > Hi Thomas,
> > 
> > On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote:
> > > Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from
> > > fbdev and drivers, as briefly discussed at [1]. Both flags were maybe
> > > useful when fbdev had special handling for driver modules. With
> > > commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0
> > > and have no further effect.
> > > 
> > > Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5
> > > split this by the way the fb_info struct is being allocated. All flags
> > > are cleared to zero during the allocation.
> > > 
> > > Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes
> > > an actual bug in how arch/sh uses the tokne for struct fb_videomode,
> > > which is unrelated.
> > > 
> > > Patch 17 removes both flag constants from 
> > 
> > We have a few more flags that are unused - should they be nuked too?
> > FBINFO_HWACCEL_FILLRECT
> > FBINFO_HWACCEL_ROTATE
> > FBINFO_HWACCEL_XPAN
> 
> It seems those are there for completeness. Nothing sets _ROTATE, the others
> are simply never checked. According to the comments, some are required, some
> are optional. I don't know what that means.
> 
> IIRC there were complains about performance when Daniel tried to remove
> fbcon acceleration, so not all _HWACCEL_ flags are unneeded.
> 
> Leaving them in for reference/completeness might be an option; or not. I
> have no strong feelings about those flags.
> 
> > 
> > Unused as in no references from fbdev/core/*
> > 
> > I would rather see one series nuke all unused FBINFO flags in one go.
> > Assuming my quick grep are right and the above can be dropped.
> 
> I would not want to extend this series. I'm removing _DEFAULT as it's
> absolutely pointless and confusing.

OK, makes sense and thanks for the explanation.

The series is:
Acked-by: Sam Ravnborg 

Re: [PATCH 00/17] fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags

[Sam Ravnborg]

Hi Thomas,

On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote:
> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from
> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe
> useful when fbdev had special handling for driver modules. With
> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0
> and have no further effect.
> 
> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5
> split this by the way the fb_info struct is being allocated. All flags
> are cleared to zero during the allocation.
> 
> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes
> an actual bug in how arch/sh uses the tokne for struct fb_videomode,
> which is unrelated.
> 
> Patch 17 removes both flag constants from 

We have a few more flags that are unused - should they be nuked too?
FBINFO_HWACCEL_FILLRECT
FBINFO_HWACCEL_ROTATE
FBINFO_HWACCEL_XPAN

Unused as in no references from fbdev/core/*

I would rather see one series nuke all unused FBINFO flags in one go.
Assuming my quick grep are right and the above can be dropped.

Sam

Re: [PATCH 12/15] auxdisplay: ht16k33: Introduce backlight_get_brightness()

[Sam Ravnborg]

On Sun, Mar 19, 2023 at 02:44:08PM +0100, Stephen Kitt wrote:
> Hi,
> 
> On Mon, 09 Jan 2023 11:12:02 +0100, Robin van der Gracht 
> wrote:
> > On 2023-01-08 10:29, Sam Ravnborg wrote:
> > > On Sat, Jan 07, 2023 at 10:02:38PM +0100, Miguel Ojeda wrote:  
> > >> On Sat, Jan 7, 2023 at 7:26 PM Sam Ravnborg via B4 Submission Endpoint
> > >>  wrote:  
> > >> >
> > >> > Introduce backlight_get_brightness() to simplify logic
> > >> > and avoid direct access to backlight properties.  
> > >> 
> > >> Note: Stephen sent this one too a while ago (with some more details in
> > >> the commit message, which is always nice); and then he sent yesterday
> > >> v2 [1] (to mention the functional change with `BL_CORE_SUSPENDED`
> > >> [2]).  
> > > Thanks for the pointers. I will try to move forward with Stephen's
> > > patches.  
> > >> 
> > >> Anyway, if it goes via drm-misc, feel free to have my:
> > >> 
> > >> Acked-by: Miguel Ojeda 
> > >> 
> > >> Though it would be nice to have Robin test the change.  
> > > 
> > > Robin - can I get your ack to apply Stephen's original v2 patch to
> > > drm-misc?  
> > 
> > done! see: 
> > https://lore.kernel.org/lkml/0b16391f997e6ed005a326e4e48f2...@protonic.nl/
> 
> As far as I can tell, this never got applied to drm-misc, and I don’t see it
> anywhere else. I guess it slipped through the cracks ;-)

Yes, I have been busy with a lot of other stuff lately, and cannot
promise when I get back to do Linux work.
So if someone else could pick it up that would be nice.

Sam

Re: [PATCH 15/15] backlight: backlight: Drop the deprecated fb_blank property

[Sam Ravnborg]

Hi Lee,
On Thu, Jan 26, 2023 at 02:27:26PM +, Lee Jones wrote:
> On Sat, 07 Jan 2023, Sam Ravnborg via B4 Submission Endpoint wrote:
> 
> > From: Sam Ravnborg 
> > 
> > With all users gone remove the deprecated fb_blank member in
> > backlight_properties.
> > 
> > Signed-off-by: Sam Ravnborg 
> > Cc: Lee Jones 
> > Cc: Daniel Thompson 
> > Cc: Jingoo Han 
> > ---
> >  drivers/video/backlight/backlight.c |  2 --
> >  include/linux/backlight.h   | 22 --
> >  2 files changed, 24 deletions(-)
> 
> Applied, thanks

Some of the dependent patches in this series are not yet applied.
I have them queued up for processing this weekend, but I missed the -rc6
window for drm-misc so they will likely not hit upstream in the upcoming
merge window.
I can try to expedite it.

But if you have not yet pushed it, please revert this patch.
Then I will resend only when the remaining patches are upstream.

Sam

Re: [PATCH 02/15] video: fbdev: atyfb: Introduce backlight_get_brightness()

[Sam Ravnborg]

Hi Christophe,
On Mon, Jan 09, 2023 at 05:44:46PM +, Christophe Leroy wrote:
> 
> 
> Le 07/01/2023 à 19:26, Sam Ravnborg via B4 Submission Endpoint a écrit :
> > From: Sam Ravnborg 
> > 
> > Introduce backlight_get_brightness() to simplify logic
> > and avoid direct access to backlight properties.
> 
> When I read 'introduce' I understand that you are adding a new function.
> 
> In fact backlight_get_brightness() already exists, so maybe replace 
> 'introduce' by 'use'

Thanks for your feedback. A similar patch is already applied to the
fbdev tree, so this patch can be ignored.

Sam

Re: [PATCH 15/15] backlight: backlight: Drop the deprecated fb_blank property

[Sam Ravnborg]

Hi Daniel.

On Mon, Jan 09, 2023 at 11:06:35AM +, Daniel Thompson wrote:
> On Sat, Jan 07, 2023 at 07:26:29PM +0100, Sam Ravnborg via B4 Submission 
> Endpoint wrote:
> > From: Sam Ravnborg 
> >
> > With all users gone remove the deprecated fb_blank member in
> > backlight_properties.
> >
> > Signed-off-by: Sam Ravnborg 
> > Cc: Lee Jones 
> > Cc: Daniel Thompson 
> > Cc: Jingoo Han 
> 
> 
> Reviewed-by: Daniel Thompson 

Thanks for the follow-up on all the backlight related patches.

> 
> 
> PS Please don't treat this like a maintainer Acked-by: and merge it
>(Lee's not on holiday so work with Lee to figure out the merge
>strategy ;-) ).
Nope, I am aware that the usual pattern here and wait for Lee to show
up.
For this patch there is a bug as I need to update a comment.
I will fix this when I resend after all the patches in flight has
landed. So likely after the next merge window,

Sam

Re: [PATCH 09/15] staging: fbtft: fb_ssd1351.c: Introduce backlight_is_blank()

[Sam Ravnborg]

Hi Stephen,

On Sun, Jan 08, 2023 at 08:28:17PM +0100, Stephen Kitt wrote:
> On Sat, 07 Jan 2023 19:26:23 +0100, Sam Ravnborg via B4 Submission Endpoint
>  wrote:
> 
> > From: Sam Ravnborg 
> > 
> > Avoiding direct access to backlight_properties.props.
> > 
> > Access to the deprecated props.fb_blank replaced by backlight_is_blank().
> > Access to props.power is dropped - it was only used for debug.
> > 
> > Signed-off-by: Sam Ravnborg 
> > Cc: Stephen Kitt 
> > Cc: Greg Kroah-Hartman 
> > Cc: Daniel Thompson 
> > Cc: Andy Shevchenko 
> > Cc: linux-fb...@vger.kernel.org
> > ---
> >  drivers/staging/fbtft/fb_ssd1351.c | 9 +++--
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/staging/fbtft/fb_ssd1351.c
> > b/drivers/staging/fbtft/fb_ssd1351.c index b8d55aa8c5c7..995fbd2f3dc6 100644
> > --- a/drivers/staging/fbtft/fb_ssd1351.c
> > +++ b/drivers/staging/fbtft/fb_ssd1351.c
> > @@ -190,15 +190,12 @@ static struct fbtft_display display = {
> >  static int update_onboard_backlight(struct backlight_device *bd)
> >  {
> > struct fbtft_par *par = bl_get_data(bd);
> > -   bool on;
> > +   bool blank = backlight_is_blank(bd);
> >  
> > -   fbtft_par_dbg(DEBUG_BACKLIGHT, par,
> > - "%s: power=%d, fb_blank=%d\n",
> > - __func__, bd->props.power, bd->props.fb_blank);
> > +   fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s: blank=%d\n", __func__,
> > blank); 
> > -   on = !backlight_is_blank(bd);
> > /* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
> > -   write_reg(par, 0xB5, on ? 0x03 : 0x02);
> > +   write_reg(par, 0xB5, !blank ? 0x03 : 0x02);
> >  
> > return 0;
> >  }
> > 
> > -- 
> > 2.34.1
> 
> For debugging purposes here, would there be any point in logging props.state?
> As in
> 
> fbtft_par_dbg(DEBUG_BACKLIGHT, par,
> - "%s: power=%d, fb_blank=%d\n",
> - __func__, bd->props.power, bd->props.fb_blank);
> + "%s: power=%d, state=%u\n",
> + __func__, bd->props.power, bd->props.state);

Thanks for the suggestion - and the reviews!

I was tempted to just remove the debugging.
If we require debugging, then this could be added in the backlight core,
thus everyone would benefit from it.

The solution above avoid any direct use of backlight_properties
which I consider a layer violation outside the backlight core.
(We cannot avoid it today with the current interface - but we can
minimize it).

Sam

Re: [PATCH 01/15] video: fbdev: atmel_lcdfb: Rework backlight handling

[Sam Ravnborg]

Hi Helge,

> > 
> > Helge - could you pick the reviewed patches from:
> > https://lore.kernel.org/all/20220607192335.1137249-1-st...@sk2.org/
> > [This is the same mail as Stephen refer to above - looked up via lore].
> 
> I just pulled those 7 patches into fbdev/for-next.
> If you need more, please let me know,

Thanks, we have one pending patch for atmel_lcdfb, but it need a small
adjustment before it is ready.

With this all fbdev drivers have the backlight_properties.fb_blank
removed.

Sam

Re: [PATCH 01/15] video: fbdev: atmel_lcdfb: Rework backlight handling

[Sam Ravnborg]

Hi Stephen.

 
> Here are my pending patches from last June on lore:
> 

All patches are handled I think except this:
> * https://lore.kernel.org/lkml/20220608205623.2106113-1-st...@sk2.org/

Can I ask you to drop the assignment that is not needed, and resend with
the collected acks/r-b.

With this, then all fbdev patches are handled.

Sam

Re: [PATCH 12/15] auxdisplay: ht16k33: Introduce backlight_get_brightness()

[Sam Ravnborg]

Hi Robin.

On Sat, Jan 07, 2023 at 10:02:38PM +0100, Miguel Ojeda wrote:
> On Sat, Jan 7, 2023 at 7:26 PM Sam Ravnborg via B4 Submission Endpoint
>  wrote:
> >
> > Introduce backlight_get_brightness() to simplify logic
> > and avoid direct access to backlight properties.
> 
> Note: Stephen sent this one too a while ago (with some more details in
> the commit message, which is always nice); and then he sent yesterday
> v2 [1] (to mention the functional change with `BL_CORE_SUSPENDED`
> [2]).
Thanks for the pointers. I will try to move forward with Stephen's
patches.
> 
> Anyway, if it goes via drm-misc, feel free to have my:
> 
> Acked-by: Miguel Ojeda 
> 
> Though it would be nice to have Robin test the change.

Robin - can I get your ack to apply Stephen's original v2 patch to
drm-misc?

Sam

> 
> Thanks!
> 
> [1] https://lore.kernel.org/lkml/20230106143002.1434266-1-st...@sk2.org/
> [2] 
> https://lore.kernel.org/lkml/caniq72krhmt37h1fagygny83onyxeqjuo8zpbym0ajqowky...@mail.gmail.com/
> 
> Cheers,
> Miguel

[PATCH 10/15] staging: fbtft: core: Introduce backlight_is_blank()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Avoiding direct access to backlight_properties.props.

Access to the deprecated props.fb_blank replaced by backlight_is_blank().
Access to props.power is dropped - it was only used for debug.

Signed-off-by: Sam Ravnborg 
Cc: Thomas Zimmermann 
Cc: Andy Shevchenko 
Cc: Javier Martinez Canillas 
Cc: Greg Kroah-Hartman 
Cc: Sam Ravnborg 
Cc: Stephen Kitt 
Cc: Peter Suti 
Cc: linux-fb...@vger.kernel.org
---
 drivers/staging/fbtft/fbtft-core.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index afaba94d1d1c..1746327e1939 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -132,15 +132,15 @@ static int fbtft_backlight_update_status(struct 
backlight_device *bd)
 {
struct fbtft_par *par = bl_get_data(bd);
bool polarity = par->polarity;
+   bool blank = backlight_is_blank(bd);
 
-   fbtft_par_dbg(DEBUG_BACKLIGHT, par,
- "%s: polarity=%d, power=%d, fb_blank=%d\n",
- __func__, polarity, bd->props.power, bd->props.fb_blank);
+   fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s: polarity=%d, blank=%d\n",
+ __func__, polarity, blank);
 
-   if (!backlight_is_blank(bd))
-   gpiod_set_value(par->gpio.led[0], polarity);
-   else
+   if (blank)
gpiod_set_value(par->gpio.led[0], !polarity);
+   else
+   gpiod_set_value(par->gpio.led[0], polarity);
 
return 0;
 }

-- 
2.34.1

[PATCH 15/15] backlight: backlight: Drop the deprecated fb_blank property

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

With all users gone remove the deprecated fb_blank member in
backlight_properties.

Signed-off-by: Sam Ravnborg 
Cc: Lee Jones 
Cc: Daniel Thompson 
Cc: Jingoo Han 
---
 drivers/video/backlight/backlight.c |  2 --
 include/linux/backlight.h   | 22 --
 2 files changed, 24 deletions(-)

diff --git a/drivers/video/backlight/backlight.c 
b/drivers/video/backlight/backlight.c
index b788ff3d0f45..9b0557d094c5 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -118,14 +118,12 @@ static int fb_notifier_callback(struct notifier_block 
*self,
bd->fb_bl_on[node] = true;
if (!bd->use_count++) {
bd->props.state &= ~BL_CORE_FBBLANK;
-   bd->props.fb_blank = FB_BLANK_UNBLANK;
backlight_update_status(bd);
}
} else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
bd->fb_bl_on[node] = false;
if (!(--bd->use_count)) {
bd->props.state |= BL_CORE_FBBLANK;
-   bd->props.fb_blank = fb_blank;
backlight_update_status(bd);
}
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 614653e07e3a..c8622d6cc8c5 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -218,25 +218,6 @@ struct backlight_properties {
 */
int power;
 
-   /**
-* @fb_blank: The power state from the FBIOBLANK ioctl.
-*
-* When the FBIOBLANK ioctl is called @fb_blank is set to the
-* blank parameter and the update_status() operation is called.
-*
-* When the backlight device is enabled @fb_blank is set
-* to FB_BLANK_UNBLANK. When the backlight device is disabled
-* @fb_blank is set to FB_BLANK_POWERDOWN.
-*
-* Backlight drivers should avoid using this property. It has been
-* replaced by state & BL_CORE_FBLANK (although most drivers should
-* use backlight_is_blank() as the preferred means to get the blank
-* state).
-*
-* fb_blank is deprecated and will be removed.
-*/
-   int fb_blank;
-
/**
 * @type: The type of backlight supported.
 *
@@ -366,7 +347,6 @@ static inline int backlight_enable(struct backlight_device 
*bd)
return 0;
 
bd->props.power = FB_BLANK_UNBLANK;
-   bd->props.fb_blank = FB_BLANK_UNBLANK;
bd->props.state &= ~BL_CORE_FBBLANK;
 
return backlight_update_status(bd);
@@ -382,7 +362,6 @@ static inline int backlight_disable(struct backlight_device 
*bd)
return 0;
 
bd->props.power = FB_BLANK_POWERDOWN;
-   bd->props.fb_blank = FB_BLANK_POWERDOWN;
bd->props.state |= BL_CORE_FBBLANK;
 
return backlight_update_status(bd);
@@ -403,7 +382,6 @@ static inline int backlight_disable(struct backlight_device 
*bd)
 static inline bool backlight_is_blank(const struct backlight_device *bd)
 {
return bd->props.power != FB_BLANK_UNBLANK ||
-  bd->props.fb_blank != FB_BLANK_UNBLANK ||
   bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK);
 }
 

-- 
2.34.1

[PATCH 12/15] auxdisplay: ht16k33: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Robin van der Gracht 
Cc: Miguel Ojeda 
Cc: Geert Uytterhoeven 
---
 drivers/auxdisplay/ht16k33.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 02425991c159..15ab118c80f5 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -314,14 +314,9 @@ static int ht16k33_initialize(struct ht16k33_priv *priv)
 
 static int ht16k33_bl_update_status(struct backlight_device *bl)
 {
-   int brightness = bl->props.brightness;
+   int brightness = backlight_get_brightness(bl);
struct ht16k33_priv *priv = bl_get_data(bl);
 
-   if (bl->props.power != FB_BLANK_UNBLANK ||
-   bl->props.fb_blank != FB_BLANK_UNBLANK ||
-   bl->props.state & BL_CORE_FBBLANK)
-   brightness = 0;
-
return ht16k33_brightness_set(priv, brightness);
 }
 

-- 
2.34.1

[PATCH 13/15] backlight: omap1: Use backlight helpers

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Rework backlight handling to avoid access to the deprecated
backlight_properties.fb_blank member.

The rework includes removal of get_brightness() operation,
because there was no read back from HW so no use for it.

Signed-off-by: Sam Ravnborg 
Cc: Lee Jones 
Cc: Daniel Thompson 
Cc: Jingoo Han 
---
 drivers/video/backlight/omap1_bl.c | 67 +-
 1 file changed, 9 insertions(+), 58 deletions(-)

diff --git a/drivers/video/backlight/omap1_bl.c 
b/drivers/video/backlight/omap1_bl.c
index 69a49384b3de..49f37da857e7 100644
--- a/drivers/video/backlight/omap1_bl.c
+++ b/drivers/video/backlight/omap1_bl.c
@@ -20,9 +20,6 @@
 #define OMAPBL_MAX_INTENSITY   0xff
 
 struct omap_backlight {
-   int powermode;
-   int current_intensity;
-
struct device *dev;
struct omap_backlight_config *pdata;
 };
@@ -37,82 +34,40 @@ static inline void omapbl_send_enable(int enable)
omap_writeb(enable, OMAP_PWL_CLK_ENABLE);
 }
 
-static void omapbl_blank(struct omap_backlight *bl, int mode)
-{
-   if (bl->pdata->set_power)
-   bl->pdata->set_power(bl->dev, mode);
-
-   switch (mode) {
-   case FB_BLANK_NORMAL:
-   case FB_BLANK_VSYNC_SUSPEND:
-   case FB_BLANK_HSYNC_SUSPEND:
-   case FB_BLANK_POWERDOWN:
-   omapbl_send_intensity(0);
-   omapbl_send_enable(0);
-   break;
-
-   case FB_BLANK_UNBLANK:
-   omapbl_send_intensity(bl->current_intensity);
-   omapbl_send_enable(1);
-   break;
-   }
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int omapbl_suspend(struct device *dev)
 {
struct backlight_device *bl_dev = dev_get_drvdata(dev);
-   struct omap_backlight *bl = bl_get_data(bl_dev);
 
-   omapbl_blank(bl, FB_BLANK_POWERDOWN);
+   backlight_disable(bl_dev);
return 0;
 }
 
 static int omapbl_resume(struct device *dev)
 {
struct backlight_device *bl_dev = dev_get_drvdata(dev);
-   struct omap_backlight *bl = bl_get_data(bl_dev);
 
-   omapbl_blank(bl, bl->powermode);
+   backlight_enable(bl_dev);
return 0;
 }
 #endif
 
-static int omapbl_set_power(struct backlight_device *dev, int state)
-{
-   struct omap_backlight *bl = bl_get_data(dev);
-
-   omapbl_blank(bl, state);
-   bl->powermode = state;
-
-   return 0;
-}
-
 static int omapbl_update_status(struct backlight_device *dev)
 {
-   struct omap_backlight *bl = bl_get_data(dev);
+   int brightness = backlight_get_brightness(dev);
 
-   if (bl->current_intensity != dev->props.brightness) {
-   if (bl->powermode == FB_BLANK_UNBLANK)
-   omapbl_send_intensity(dev->props.brightness);
-   bl->current_intensity = dev->props.brightness;
+   if (brightness > 0) {
+   omapbl_send_intensity(dev->props.brightness);
+   omapbl_send_enable(1);
+   } else {
+   omapbl_send_intensity(0);
+   omapbl_send_enable(0);
}
 
-   if (dev->props.fb_blank != bl->powermode)
-   omapbl_set_power(dev, dev->props.fb_blank);
-
return 0;
 }
 
-static int omapbl_get_intensity(struct backlight_device *dev)
-{
-   struct omap_backlight *bl = bl_get_data(dev);
-
-   return bl->current_intensity;
-}
-
 static const struct backlight_ops omapbl_ops = {
-   .get_brightness = omapbl_get_intensity,
.update_status  = omapbl_update_status,
 };
 
@@ -139,9 +94,6 @@ static int omapbl_probe(struct platform_device *pdev)
if (IS_ERR(dev))
return PTR_ERR(dev);
 
-   bl->powermode = FB_BLANK_POWERDOWN;
-   bl->current_intensity = 0;
-
bl->pdata = pdata;
bl->dev = >dev;
 
@@ -149,7 +101,6 @@ static int omapbl_probe(struct platform_device *pdev)
 
omap_cfg_reg(PWL);  /* Conflicts with UART3 */
 
-   dev->props.fb_blank = FB_BLANK_UNBLANK;
dev->props.brightness = pdata->default_intensity;
omapbl_update_status(dev);
 

-- 
2.34.1

[PATCH 14/15] backlight: tosa: Use backlight helper

[Sam Ravnborg via B4 Submission Endpoint]

From: Stephen Kitt 

Instead of retrieving the backlight brightness in struct
backlight_properties manually, and then checking whether the backlight
should be on at all, use backlight_get_brightness() which does all
this and insulates this from future changes.

Signed-off-by: Stephen Kitt 
Signed-off-by: Sam Ravnborg 
---
 drivers/video/backlight/tosa_bl.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/video/backlight/tosa_bl.c 
b/drivers/video/backlight/tosa_bl.c
index f55b3d616a87..cb07f13dd886 100644
--- a/drivers/video/backlight/tosa_bl.c
+++ b/drivers/video/backlight/tosa_bl.c
@@ -50,13 +50,8 @@ static void tosa_bl_set_backlight(struct tosa_bl_data *data, 
int brightness)
 
 static int tosa_bl_update_status(struct backlight_device *dev)
 {
-   struct backlight_properties *props = >props;
struct tosa_bl_data *data = bl_get_data(dev);
-   int power = max(props->power, props->fb_blank);
-   int brightness = props->brightness;
-
-   if (power)
-   brightness = 0;
+   int brightness = backlight_get_brightness(dev);
 
tosa_bl_set_backlight(data, brightness);
 

-- 
2.34.1

[PATCH 11/15] powerpc: via-pmu-backlight: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Benjamin Herrenschmidt 
Cc: Sam Ravnborg 
Cc: linuxppc-dev@lists.ozlabs.org
---
 drivers/macintosh/via-pmu-backlight.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/macintosh/via-pmu-backlight.c 
b/drivers/macintosh/via-pmu-backlight.c
index 2194016122d2..c2d87e7fa85b 100644
--- a/drivers/macintosh/via-pmu-backlight.c
+++ b/drivers/macintosh/via-pmu-backlight.c
@@ -71,12 +71,7 @@ static int pmu_backlight_get_level_brightness(int level)
 static int __pmu_backlight_update_status(struct backlight_device *bd)
 {
struct adb_request req;
-   int level = bd->props.brightness;
-
-
-   if (bd->props.power != FB_BLANK_UNBLANK ||
-   bd->props.fb_blank != FB_BLANK_UNBLANK)
-   level = 0;
+   int level = backlight_get_brightness(bd);
 
if (level > 0) {
int pmulevel = pmu_backlight_get_level_brightness(level);

-- 
2.34.1

[PATCH 08/15] video: fbdev: omap2: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Allison Randal 
Cc: Sam Ravnborg 
Cc: Greg Kroah-Hartman 
Cc: Kate Stewart 
Cc: Thomas Gleixner 
Cc: Enrico Weigelt 
Cc: Alexios Zavras 
---
 .../fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 19 +-
 .../omap2/omapfb/displays/panel-sony-acx565akm.c   | 23 +++---
 2 files changed, 8 insertions(+), 34 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c 
b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
index a2c7c5cb1523..bd73aa5328c9 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
@@ -330,14 +330,8 @@ static int dsicm_bl_update_status(struct backlight_device 
*dev)
 {
struct panel_drv_data *ddata = dev_get_drvdata(>dev);
struct omap_dss_device *in = ddata->in;
+   int level = backlight_get_brightness(dev);
int r;
-   int level;
-
-   if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-   dev->props.power == FB_BLANK_UNBLANK)
-   level = dev->props.brightness;
-   else
-   level = 0;
 
dev_dbg(>pdev->dev, "update brightness to %d\n", level);
 
@@ -360,17 +354,7 @@ static int dsicm_bl_update_status(struct backlight_device 
*dev)
return r;
 }
 
-static int dsicm_bl_get_intensity(struct backlight_device *dev)
-{
-   if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-   dev->props.power == FB_BLANK_UNBLANK)
-   return dev->props.brightness;
-
-   return 0;
-}
-
 static const struct backlight_ops dsicm_bl_ops = {
-   .get_brightness = dsicm_bl_get_intensity,
.update_status  = dsicm_bl_update_status,
 };
 
@@ -1251,7 +1235,6 @@ static int dsicm_probe(struct platform_device *pdev)
 
ddata->bldev = bldev;
 
-   bldev->props.fb_blank = FB_BLANK_UNBLANK;
bldev->props.power = FB_BLANK_UNBLANK;
bldev->props.brightness = 255;
 
diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c 
b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
index c0965bee12c5..c9c8f10e2e2f 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
@@ -337,16 +337,10 @@ static int acx565akm_get_actual_brightness(struct 
panel_drv_data *ddata)
 static int acx565akm_bl_update_status(struct backlight_device *dev)
 {
struct panel_drv_data *ddata = dev_get_drvdata(>dev);
-   int level;
+   int level = backlight_get_brightness(dev);
 
dev_dbg(>spi->dev, "%s\n", __func__);
 
-   if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-   dev->props.power == FB_BLANK_UNBLANK)
-   level = dev->props.brightness;
-   else
-   level = 0;
-
if (ddata->has_bc)
acx565akm_set_brightness(ddata, level);
else
@@ -364,15 +358,13 @@ static int acx565akm_bl_get_intensity(struct 
backlight_device *dev)
if (!ddata->has_bc)
return -ENODEV;
 
-   if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
-   dev->props.power == FB_BLANK_UNBLANK) {
-   if (ddata->has_bc)
-   return acx565akm_get_actual_brightness(ddata);
-   else
-   return dev->props.brightness;
-   }
+   if (backlight_is_blank(dev))
+   return 0;
 
-   return 0;
+   if (ddata->has_bc)
+   return acx565akm_get_actual_brightness(ddata);
+   else
+   return backlight_get_brightness(dev);
 }
 
 static int acx565akm_bl_update_status_locked(struct backlight_device *dev)
@@ -795,7 +787,6 @@ static int acx565akm_probe(struct spi_device *spi)
}
 
memset(, 0, sizeof(props));
-   props.fb_blank = FB_BLANK_UNBLANK;
props.power = FB_BLANK_UNBLANK;
props.type = BACKLIGHT_RAW;
 

-- 
2.34.1

[PATCH 06/15] video: fbdev: aty128fb: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Paul Mackerras 
Cc: linux-fb...@vger.kernel.org
---
 drivers/video/fbdev/aty/aty128fb.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/aty/aty128fb.c 
b/drivers/video/fbdev/aty/aty128fb.c
index dd31b9d7d337..736126cc5049 100644
--- a/drivers/video/fbdev/aty/aty128fb.c
+++ b/drivers/video/fbdev/aty/aty128fb.c
@@ -1764,17 +1764,10 @@ static int aty128_bl_update_status(struct 
backlight_device *bd)
 {
struct aty128fb_par *par = bl_get_data(bd);
unsigned int reg = aty_ld_le32(LVDS_GEN_CNTL);
-   int level;
-
-   if (bd->props.power != FB_BLANK_UNBLANK ||
-   bd->props.fb_blank != FB_BLANK_UNBLANK ||
-   !par->lcd_on)
-   level = 0;
-   else
-   level = bd->props.brightness;
+   int level = backlight_get_brightness(bd);
 
reg |= LVDS_BL_MOD_EN | LVDS_BLON;
-   if (level > 0) {
+   if (level > 0 || par->lcd_on) {
reg |= LVDS_DIGION;
if (!(reg & LVDS_ON)) {
reg &= ~LVDS_BLON;

-- 
2.34.1

[PATCH 03/15] video: fbdev: nvidia: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Antonino Daplas 
Cc: linux-fb...@vger.kernel.org
---
 drivers/video/fbdev/nvidia/nv_backlight.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/nvidia/nv_backlight.c 
b/drivers/video/fbdev/nvidia/nv_backlight.c
index 2ce53529f636..503a7a683855 100644
--- a/drivers/video/fbdev/nvidia/nv_backlight.c
+++ b/drivers/video/fbdev/nvidia/nv_backlight.c
@@ -49,17 +49,11 @@ static int nvidia_bl_update_status(struct backlight_device 
*bd)
 {
struct nvidia_par *par = bl_get_data(bd);
u32 tmp_pcrt, tmp_pmc, fpcontrol;
-   int level;
+   int level = backlight_get_brightness(bd);
 
if (!par->FlatPanel)
return 0;
 
-   if (bd->props.power != FB_BLANK_UNBLANK ||
-   bd->props.fb_blank != FB_BLANK_UNBLANK)
-   level = 0;
-   else
-   level = bd->props.brightness;
-
tmp_pmc = NV_RD32(par->PMC, 0x10F0) & 0x;
tmp_pcrt = NV_RD32(par->PCRTC0, 0x081C) & 0xFFFC;
fpcontrol = NV_RD32(par->PRAMDAC, 0x0848) & 0xCFCC;

-- 
2.34.1

[PATCH 00/15] backlight: Drop use of deprecated fb_blank property

[Sam Ravnborg via B4 Submission Endpoint]

This series refactor backlight users to avoid use of the
deprecated backlight_properties.fb_blank member.

Stephen Kitt  and others already did a lot of
work and this is the final touches.

Patches 1-13 are independent and can be applied individually.
Patch 14 was already sent by Stephen and included here to make
the series complete.

The last patch may have to wait to avoid breaking the build
as it depends on all the other patches.

The series touches several sub-systems, so with acks I could
take them all in drm-misc. Or we can let the subsystems take
them and wait until next merge window with the final removal.
As new users of fb_blank do not pop up that often, waiting
one merge cycle is fine.

Sam

To: Nicolas Ferre 
To: Helge Deller 
To: Alexandre Belloni 
To: Claudiu Beznea 
To: Antonino Daplas 
To: Benjamin Herrenschmidt 
To: Paul Mackerras 
To: Greg Kroah-Hartman 
To: Robin van der Gracht 
To: Miguel Ojeda 
To: Lee Jones 
To: Daniel Thompson 
To: Jingoo Han 
Cc: linux-fb...@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-stag...@lists.linux.dev
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Stephen Kitt 
Signed-off-by: Sam Ravnborg 

---
Sam Ravnborg (14):
  video: fbdev: atmel_lcdfb: Rework backlight handling
  video: fbdev: atyfb: Introduce backlight_get_brightness()
  video: fbdev: nvidia: Introduce backlight_get_brightness()
  video: fbdev: radeon: Introduce backlight_get_brightness()
  video: fbdev: riva: Introduce backlight_get_brightness()
  video: fbdev: aty128fb: Introduce backlight_get_brightness()
  video: fbdev: mx3fb: Introduce backlight_get_brightness()
  video: fbdev: omap2: Introduce backlight_get_brightness()
  staging: fbtft: fb_ssd1351.c: Introduce backlight_is_blank()
  staging: fbtft: core: Introduce backlight_is_blank()
  powerpc: via-pmu-backlight: Introduce backlight_get_brightness()
  auxdisplay: ht16k33: Introduce backlight_get_brightness()
  backlight: omap1: Use backlight helpers
  backlight: backlight: Drop the deprecated fb_blank property

Stephen Kitt (1):
  backlight: tosa: Use backlight helper

 drivers/auxdisplay/ht16k33.c   |  7 +--
 drivers/macintosh/via-pmu-backlight.c  |  7 +--
 drivers/staging/fbtft/fb_ssd1351.c |  9 +--
 drivers/staging/fbtft/fbtft-core.c | 12 ++--
 drivers/video/backlight/backlight.c|  2 -
 drivers/video/backlight/omap1_bl.c | 67 +++---
 drivers/video/backlight/tosa_bl.c  |  7 +--
 drivers/video/fbdev/atmel_lcdfb.c  | 24 +---
 drivers/video/fbdev/aty/aty128fb.c | 11 +---
 drivers/video/fbdev/aty/atyfb_base.c   |  8 +--
 drivers/video/fbdev/aty/radeon_backlight.c | 10 +---
 drivers/video/fbdev/mx3fb.c|  8 +--
 drivers/video/fbdev/nvidia/nv_backlight.c  |  8 +--
 .../fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 19 +-
 .../omap2/omapfb/displays/panel-sony-acx565akm.c   | 23 +++-
 drivers/video/fbdev/riva/fbdev.c   |  8 +--
 include/linux/backlight.h  | 22 ---
 17 files changed, 41 insertions(+), 211 deletions(-)
---
base-commit: a53be8dae86fe5d3567db245177e814e58210632
change-id: 20230107-sam-video-backlight-drop-fb_blank-d6feb73572ff

Best regards,
-- 
Sam Ravnborg 

[PATCH 01/15] video: fbdev: atmel_lcdfb: Rework backlight handling

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

The atmel_lcdfb had code to save/restore power state.
This is not needed so drop it.

Introduce backlight_is_brightness() to make logic simpler.

Signed-off-by: Sam Ravnborg 
Cc: Nicolas Ferre 
Cc: Alexandre Belloni 
Cc: Ludovic Desroches 
Cc: linux-fb...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
---
 drivers/video/fbdev/atmel_lcdfb.c | 24 +++-
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/video/fbdev/atmel_lcdfb.c 
b/drivers/video/fbdev/atmel_lcdfb.c
index 1fc8de4ecbeb..d297b3892637 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -49,7 +49,6 @@ struct atmel_lcdfb_info {
struct clk  *lcdc_clk;
 
struct backlight_device *backlight;
-   u8  bl_power;
u8  saved_lcdcon;
 
u32 pseudo_palette[16];
@@ -109,32 +108,18 @@ static u32 contrast_ctr = ATMEL_LCDC_PS_DIV8
 static int atmel_bl_update_status(struct backlight_device *bl)
 {
struct atmel_lcdfb_info *sinfo = bl_get_data(bl);
-   int power = sinfo->bl_power;
-   int brightness = bl->props.brightness;
+   int brightness;
 
-   /* REVISIT there may be a meaningful difference between
-* fb_blank and power ... there seem to be some cases
-* this doesn't handle correctly.
-*/
-   if (bl->props.fb_blank != sinfo->bl_power)
-   power = bl->props.fb_blank;
-   else if (bl->props.power != sinfo->bl_power)
-   power = bl->props.power;
-
-   if (brightness < 0 && power == FB_BLANK_UNBLANK)
-   brightness = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
-   else if (power != FB_BLANK_UNBLANK)
-   brightness = 0;
+   brightness = backlight_get_brightness(bl);
 
lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, brightness);
+
if (contrast_ctr & ATMEL_LCDC_POL_POSITIVE)
lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR,
brightness ? contrast_ctr : 0);
else
lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, contrast_ctr);
 
-   bl->props.fb_blank = bl->props.power = sinfo->bl_power = power;
-
return 0;
 }
 
@@ -155,8 +140,6 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)
struct backlight_properties props;
struct backlight_device *bl;
 
-   sinfo->bl_power = FB_BLANK_UNBLANK;
-
if (sinfo->backlight)
return;
 
@@ -173,7 +156,6 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)
sinfo->backlight = bl;
 
bl->props.power = FB_BLANK_UNBLANK;
-   bl->props.fb_blank = FB_BLANK_UNBLANK;
bl->props.brightness = atmel_bl_get_brightness(bl);
 }
 

-- 
2.34.1

[PATCH 02/15] video: fbdev: atyfb: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Bartlomiej Zolnierkiewicz 
Cc: Sam Ravnborg 
Cc: Daniel Vetter 
Cc: Souptick Joarder 
Cc: Maarten Lankhorst 
Cc: Jason Yan 
Cc: Jani Nikula 
Cc: Arnd Bergmann 
---
 drivers/video/fbdev/aty/atyfb_base.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/aty/atyfb_base.c 
b/drivers/video/fbdev/aty/atyfb_base.c
index 0ccf5d401ecb..ca361e215904 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -2219,13 +2219,7 @@ static int aty_bl_update_status(struct backlight_device 
*bd)
 {
struct atyfb_par *par = bl_get_data(bd);
unsigned int reg = aty_ld_lcd(LCD_MISC_CNTL, par);
-   int level;
-
-   if (bd->props.power != FB_BLANK_UNBLANK ||
-   bd->props.fb_blank != FB_BLANK_UNBLANK)
-   level = 0;
-   else
-   level = bd->props.brightness;
+   int level = backlight_get_brightness(bd);
 
reg |= (BLMOD_EN | BIASMOD_EN);
if (level > 0) {

-- 
2.34.1

[PATCH 07/15] video: fbdev: mx3fb: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Sam Ravnborg 
Cc: Kate Stewart 
Cc: Thomas Gleixner 
Cc: Laurent Pinchart 
Cc: Greg Kroah-Hartman 
Cc: Arnd Bergmann 
Cc: Jani Nikula 
---
 drivers/video/fbdev/mx3fb.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/mx3fb.c b/drivers/video/fbdev/mx3fb.c
index b945b68984b9..bc35f664cbff 100644
--- a/drivers/video/fbdev/mx3fb.c
+++ b/drivers/video/fbdev/mx3fb.c
@@ -283,12 +283,7 @@ static int mx3fb_bl_get_brightness(struct backlight_device 
*bl)
 static int mx3fb_bl_update_status(struct backlight_device *bl)
 {
struct mx3fb_data *fbd = bl_get_data(bl);
-   int brightness = bl->props.brightness;
-
-   if (bl->props.power != FB_BLANK_UNBLANK)
-   brightness = 0;
-   if (bl->props.fb_blank != FB_BLANK_UNBLANK)
-   brightness = 0;
+   int brightness = backlight_get_brightness(bl);
 
fbd->backlight_level = (fbd->backlight_level & ~0xFF) | brightness;
 
@@ -325,7 +320,6 @@ static void mx3fb_init_backlight(struct mx3fb_data *fbd)
 
fbd->bl = bl;
bl->props.power = FB_BLANK_UNBLANK;
-   bl->props.fb_blank = FB_BLANK_UNBLANK;
bl->props.brightness = mx3fb_bl_get_brightness(bl);
 }
 

-- 
2.34.1

[PATCH 04/15] video: fbdev: radeon: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Benjamin Herrenschmidt 
Cc: linux-fb...@vger.kernel.org
---
 drivers/video/fbdev/aty/radeon_backlight.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_backlight.c 
b/drivers/video/fbdev/aty/radeon_backlight.c
index d2c1263ad260..22a39fea7b89 100644
--- a/drivers/video/fbdev/aty/radeon_backlight.c
+++ b/drivers/video/fbdev/aty/radeon_backlight.c
@@ -54,14 +54,10 @@ static int radeon_bl_update_status(struct backlight_device 
*bd)
return 0;
 
/* We turn off the LCD completely instead of just dimming the
-* backlight. This provides some greater power saving and the display
-* is useless without backlight anyway.
+* backlight if level < 1. This provides some greater power saving
+* and the display is useless without backlight anyway.
 */
-if (bd->props.power != FB_BLANK_UNBLANK ||
-   bd->props.fb_blank != FB_BLANK_UNBLANK)
-   level = 0;
-   else
-   level = bd->props.brightness;
+   level = backlight_get_brightness(bd);
 
del_timer_sync(>lvds_timer);
radeon_engine_idle();

-- 
2.34.1

[PATCH 09/15] staging: fbtft: fb_ssd1351.c: Introduce backlight_is_blank()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Avoiding direct access to backlight_properties.props.

Access to the deprecated props.fb_blank replaced by backlight_is_blank().
Access to props.power is dropped - it was only used for debug.

Signed-off-by: Sam Ravnborg 
Cc: Stephen Kitt 
Cc: Greg Kroah-Hartman 
Cc: Daniel Thompson 
Cc: Andy Shevchenko 
Cc: linux-fb...@vger.kernel.org
---
 drivers/staging/fbtft/fb_ssd1351.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ssd1351.c 
b/drivers/staging/fbtft/fb_ssd1351.c
index b8d55aa8c5c7..995fbd2f3dc6 100644
--- a/drivers/staging/fbtft/fb_ssd1351.c
+++ b/drivers/staging/fbtft/fb_ssd1351.c
@@ -190,15 +190,12 @@ static struct fbtft_display display = {
 static int update_onboard_backlight(struct backlight_device *bd)
 {
struct fbtft_par *par = bl_get_data(bd);
-   bool on;
+   bool blank = backlight_is_blank(bd);
 
-   fbtft_par_dbg(DEBUG_BACKLIGHT, par,
- "%s: power=%d, fb_blank=%d\n",
- __func__, bd->props.power, bd->props.fb_blank);
+   fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s: blank=%d\n", __func__, blank);
 
-   on = !backlight_is_blank(bd);
/* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
-   write_reg(par, 0xB5, on ? 0x03 : 0x02);
+   write_reg(par, 0xB5, !blank ? 0x03 : 0x02);
 
return 0;
 }

-- 
2.34.1

[PATCH 05/15] video: fbdev: riva: Introduce backlight_get_brightness()

[Sam Ravnborg via B4 Submission Endpoint]

From: Sam Ravnborg 

Introduce backlight_get_brightness() to simplify logic
and avoid direct access to backlight properties.

Signed-off-by: Sam Ravnborg 
Cc: Antonino Daplas 
Cc: linux-fb...@vger.kernel.org
---
 drivers/video/fbdev/riva/fbdev.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c
index 644278146d3b..41edc6e79460 100644
--- a/drivers/video/fbdev/riva/fbdev.c
+++ b/drivers/video/fbdev/riva/fbdev.c
@@ -293,13 +293,7 @@ static int riva_bl_update_status(struct backlight_device 
*bd)
 {
struct riva_par *par = bl_get_data(bd);
U032 tmp_pcrt, tmp_pmc;
-   int level;
-
-   if (bd->props.power != FB_BLANK_UNBLANK ||
-   bd->props.fb_blank != FB_BLANK_UNBLANK)
-   level = 0;
-   else
-   level = bd->props.brightness;
+   int level = backlight_get_brightness(bd);
 
tmp_pmc = NV_RD32(par->riva.PMC, 0x10F0) & 0x;
tmp_pcrt = NV_RD32(par->riva.PCRTC0, 0x081C) & 0xFFFC;

-- 
2.34.1

Re: [PATCH 01/15] video: fbdev: atmel_lcdfb: Rework backlight handling

[Sam Ravnborg]

Hi Stephen.

On Sat, Jan 07, 2023 at 09:36:47PM +0100, Stephen Kitt wrote:
> On 7 January 2023 19:26:15 CET, Sam Ravnborg via B4 Submission Endpoint 
>  wrote:
> >From: Sam Ravnborg 
> >
> >The atmel_lcdfb had code to save/restore power state.
> >This is not needed so drop it.
> >
> >Introduce backlight_is_brightness() to make logic simpler.
> >
> >Signed-off-by: Sam Ravnborg 
> >Cc: Nicolas Ferre 
> >Cc: Alexandre Belloni 
> >Cc: Ludovic Desroches 
> >Cc: linux-fb...@vger.kernel.org
> >Cc: linux-arm-ker...@lists.infradead.org
> >---
> > drivers/video/fbdev/atmel_lcdfb.c | 24 +++-
> > 1 file changed, 3 insertions(+), 21 deletions(-)
...
> 
> Hi Sam,
> 
> I’d submitted quite a few more of these previously (and you’d reviewed them), 
> see e.g. the thread starting at https://lkml.org/lkml/2022/6/7/4365, and 
> yesterday, https://lkml.org/lkml/2023/1/6/520, 
> https://lkml.org/lkml/2023/1/6/656, https://lkml.org/lkml/2023/1/6/970, 
> https://lkml.org/lkml/2023/1/6/643, and https://lkml.org/lkml/2023/1/6/680. 
> There are a few more, I can find them if it’s any use.

The patches from yesterday was what triggered me to resurrect an old
branch of mine where I had done something similar. I had lost all
memory of reviewing similar patches from you.


Helge - could you pick the reviewed patches from:
https://lore.kernel.org/all/20220607192335.1137249-1-st...@sk2.org/
[This is the same mail as Stephen refer to above - looked up via lore].

Stephen - I expect Daniel/Lee to take care of the patches from yesterday.
If you can look up other pending patches from you please do so, so we
can have them applied.
Preferably with links to lore - as this makes it easier to apply them.

Review of what is unique in this set would be appreciated.

Sam

Re: [PATCH V5 04/26] sparc/mm: Move protection_map[] inside the platform

[Sam Ravnborg]

Hi Anshuman,

On Mon, Jun 27, 2022 at 10:28:11AM +0530, Anshuman Khandual wrote:
> This moves protection_map[] inside the platform and while here, also enable
> ARCH_HAS_VM_GET_PAGE_PROT on 32 bit platforms via DECLARE_VM_GET_PAGE_PROT.
> 
> Cc: "David S. Miller" 
> Cc: sparcli...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Anshuman Khandual 
> ---
>  arch/sparc/Kconfig  |  2 +-
>  arch/sparc/include/asm/pgtable_32.h | 19 ---
>  arch/sparc/include/asm/pgtable_64.h | 19 ---
>  arch/sparc/mm/init_32.c | 20 
>  arch/sparc/mm/init_64.c |  3 +++
>  5 files changed, 24 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index ba449c47effd..09f868613a4d 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -13,6 +13,7 @@ config 64BIT
>  config SPARC
>   bool
>   default y
> + select ARCH_HAS_VM_GET_PAGE_PROT
>   select ARCH_MIGHT_HAVE_PC_PARPORT if SPARC64 && PCI
>   select ARCH_MIGHT_HAVE_PC_SERIO
>   select DMA_OPS
> @@ -84,7 +85,6 @@ config SPARC64
>   select PERF_USE_VMALLOC
>   select ARCH_HAVE_NMI_SAFE_CMPXCHG
>   select HAVE_C_RECORDMCOUNT
> - select ARCH_HAS_VM_GET_PAGE_PROT
>   select HAVE_ARCH_AUDITSYSCALL
>   select ARCH_SUPPORTS_ATOMIC_RMW
>   select ARCH_SUPPORTS_DEBUG_PAGEALLOC
> diff --git a/arch/sparc/include/asm/pgtable_32.h 
> b/arch/sparc/include/asm/pgtable_32.h
> index 4866625da314..8ff549004fac 100644
> --- a/arch/sparc/include/asm/pgtable_32.h
> +++ b/arch/sparc/include/asm/pgtable_32.h
> @@ -64,25 +64,6 @@ void paging_init(void);
>  
>  extern unsigned long ptr_in_current_pgd;
>  
> -/* xwr */
> -#define __P000  PAGE_NONE
> -#define __P001  PAGE_READONLY
> -#define __P010  PAGE_COPY
> -#define __P011  PAGE_COPY
> -#define __P100  PAGE_READONLY
> -#define __P101  PAGE_READONLY
> -#define __P110  PAGE_COPY
> -#define __P111  PAGE_COPY
> -
> -#define __S000   PAGE_NONE
> -#define __S001   PAGE_READONLY
> -#define __S010   PAGE_SHARED
> -#define __S011   PAGE_SHARED
> -#define __S100   PAGE_READONLY
> -#define __S101   PAGE_READONLY
> -#define __S110   PAGE_SHARED
> -#define __S111   PAGE_SHARED
> -
>  /* First physical page can be anywhere, the following is needed so that
>   * va-->pa and vice versa conversions work properly without performance
>   * hit for all __pa()/__va() operations.
> diff --git a/arch/sparc/include/asm/pgtable_64.h 
> b/arch/sparc/include/asm/pgtable_64.h
> index 4679e45c8348..a779418ceba9 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -187,25 +187,6 @@ bool kern_addr_valid(unsigned long addr);
>  #define _PAGE_SZHUGE_4U  _PAGE_SZ4MB_4U
>  #define _PAGE_SZHUGE_4V  _PAGE_SZ4MB_4V
>  
> -/* These are actually filled in at boot time by sun4{u,v}_pgprot_init() */
> -#define __P000   __pgprot(0)
> -#define __P001   __pgprot(0)
> -#define __P010   __pgprot(0)
> -#define __P011   __pgprot(0)
> -#define __P100   __pgprot(0)
> -#define __P101   __pgprot(0)
> -#define __P110   __pgprot(0)
> -#define __P111   __pgprot(0)
> -
> -#define __S000   __pgprot(0)
> -#define __S001   __pgprot(0)
> -#define __S010   __pgprot(0)
> -#define __S011   __pgprot(0)
> -#define __S100   __pgprot(0)
> -#define __S101   __pgprot(0)
> -#define __S110   __pgprot(0)
> -#define __S111   __pgprot(0)
> -
>  #ifndef __ASSEMBLY__
>  
>  pte_t mk_pte_io(unsigned long, pgprot_t, int, unsigned long);
> diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c
> index 1e9f577f084d..8693e4e28b86 100644
> --- a/arch/sparc/mm/init_32.c
> +++ b/arch/sparc/mm/init_32.c
> @@ -302,3 +302,23 @@ void sparc_flush_page_to_ram(struct page *page)
>   __flush_page_to_ram(vaddr);
>  }
>  EXPORT_SYMBOL(sparc_flush_page_to_ram);
> +
> +static pgprot_t protection_map[16] __ro_after_init = {
This can be const - like done for powerpc and others.
sparc32 and sparc64 uses each their own - and I do not see sparc32 do
any modifications to protection_map.

With this change:
Reviewed-by: Sam Ravnborg 

> + [VM_NONE]   = PAGE_NONE,
> + [VM_READ]   = PAGE_READONLY,
> + [VM_WRITE]  = PAGE_COPY,
> + [VM_WRITE | VM_READ]= PAGE_COPY,
> + [VM_EXEC]   = PAGE_READONLY,
> + [VM_EXEC | VM_

Re: [PATCH v2 15/18] sparc64: remove CONFIG_SET_FS support

[Sam Ravnborg]

Hi Arnd.

On Wed, Feb 16, 2022 at 02:13:29PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> sparc64 uses address space identifiers to differentiate between kernel
> and user space, using ASI_P for kernel threads but ASI_AIUS for normal
> user space, with the option of changing between them.
> 
> As nothing really changes the ASI any more, just hardcode ASI_AIUS
> everywhere. Kernel threads are not allowed to access __user pointers
> anyway.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  arch/sparc/include/asm/processor_64.h   |  4 
>  arch/sparc/include/asm/switch_to_64.h   |  4 +---
>  arch/sparc/include/asm/thread_info_64.h |  4 +---
>  arch/sparc/include/asm/uaccess_64.h | 20 +---
>  arch/sparc/kernel/process_64.c  | 12 
>  arch/sparc/kernel/traps_64.c|  2 --
>  arch/sparc/lib/NGmemcpy.S   |  3 +--
>  arch/sparc/mm/init_64.c |  7 ---
>  8 files changed, 8 insertions(+), 48 deletions(-)

I think you somehow missed the Kconfig change, and also the related
sparc32 change which continue to have set_fs() after this patch.


I did not manage to review the patch - as I am too unfamiliar with the
code paths and the set_fs() removal changes.

Sam

Re: [PATCH v2 18/18] uaccess: drop maining CONFIG_SET_FS users

[Sam Ravnborg]

Hi Arnd,

Fix spelling in $subject...

sparc/Kconfig b/arch/sparc/Kconfig
> index 9f6f9bce5292..9276f321b3e3 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -46,7 +46,6 @@ config SPARC
>   select LOCKDEP_SMALL if LOCKDEP
>   select NEED_DMA_MAP_STATE
>   select NEED_SG_DMA_LENGTH
> - select SET_FS
>   select TRACE_IRQFLAGS_SUPPORT
>  
>  config SPARC32
> @@ -101,6 +100,7 @@ config SPARC64
>   select HAVE_SETUP_PER_CPU_AREA
>   select NEED_PER_CPU_EMBED_FIRST_CHUNK
>   select NEED_PER_CPU_PAGE_FIRST_CHUNK
> + select SET_FS
This looks wrong - looks like some merge went wrong here.

>  
>  config ARCH_PROC_KCORE_TEXT
>   def_bool y
> diff --git a/arch/sparc/include/asm/processor_32.h 
> b/arch/sparc/include/asm/processor_32.h
> index 647bf0ac7beb..b26c35336b51 100644
> --- a/arch/sparc/include/asm/processor_32.h
> +++ b/arch/sparc/include/asm/processor_32.h
> @@ -32,10 +32,6 @@ struct fpq {
>  };
>  #endif
>  
> -typedef struct {
> - int seg;
> -} mm_segment_t;
> -
>  /* The Sparc processor specific thread struct. */
>  struct thread_struct {
>   struct pt_regs *kregs;
> @@ -50,11 +46,9 @@ struct thread_struct {
>   unsigned long   fsr;
>   unsigned long   fpqdepth;
>   struct fpq  fpqueue[16];
> - mm_segment_t current_ds;
>  };
>  
>  #define INIT_THREAD  { \
> - .current_ds = KERNEL_DS, \
>   .kregs = (struct pt_regs *)(init_stack+THREAD_SIZE)-1 \
>  }
>  
> diff --git a/arch/sparc/include/asm/uaccess_32.h 
> b/arch/sparc/include/asm/uaccess_32.h
> index 367747116260..9fd6c53644b6 100644
> --- a/arch/sparc/include/asm/uaccess_32.h
> +++ b/arch/sparc/include/asm/uaccess_32.h
> @@ -12,19 +12,6 @@
>  #include 
>  
>  #include 
> -
> -/* Sparc is not segmented, however we need to be able to fool access_ok()
> - * when doing system calls from kernel mode legitimately.
> - *
> - * "For historical reasons, these macros are grossly misnamed." -Linus
> - */
> -
> -#define KERNEL_DS   ((mm_segment_t) { 0 })
> -#define USER_DS ((mm_segment_t) { -1 })
> -
> -#define get_fs() (current->thread.current_ds)
> -#define set_fs(val)  ((current->thread.current_ds) = (val))
> -
>  #include 
>  
>  /* Uh, these should become the main single-value transfer routines..
> diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
> index 2dc0bf9fe62e..88c0c14aaff0 100644
> --- a/arch/sparc/kernel/process_32.c
> +++ b/arch/sparc/kernel/process_32.c
> @@ -300,7 +300,6 @@ int copy_thread(unsigned long clone_flags, unsigned long 
> sp, unsigned long arg,
>   extern int nwindows;
>   unsigned long psr;
>   memset(new_stack, 0, STACKFRAME_SZ + TRACEREG_SZ);
> - p->thread.current_ds = KERNEL_DS;
>   ti->kpc = (((unsigned long) ret_from_kernel_thread) - 0x8);
>   childregs->u_regs[UREG_G1] = sp; /* function */
>   childregs->u_regs[UREG_G2] = arg;
> @@ -311,7 +310,6 @@ int copy_thread(unsigned long clone_flags, unsigned long 
> sp, unsigned long arg,
>   }
>   memcpy(new_stack, (char *)regs - STACKFRAME_SZ, STACKFRAME_SZ + 
> TRACEREG_SZ);
>   childregs->u_regs[UREG_FP] = sp;
> - p->thread.current_ds = USER_DS;
>   ti->kpc = (((unsigned long) ret_from_fork) - 0x8);
>   ti->kpsr = current->thread.fork_kpsr | PSR_PIL;
>   ti->kwim = current->thread.fork_kwim;

Other than the above the sparc32 changes looks fine, and with the Kconf
stuff fixed:
Acked-by: Sam Ravnborg  # for sparc32 changes

Re: [PATCH v2 15/18] sparc64: remove CONFIG_SET_FS support

[Sam Ravnborg]

Hi Arnd,

On Wed, Feb 16, 2022 at 07:34:59PM +0100, Sam Ravnborg wrote:
> Hi Arnd.
> 
> On Wed, Feb 16, 2022 at 02:13:29PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann 
> > 
> > sparc64 uses address space identifiers to differentiate between kernel
> > and user space, using ASI_P for kernel threads but ASI_AIUS for normal
> > user space, with the option of changing between them.
> > 
> > As nothing really changes the ASI any more, just hardcode ASI_AIUS
> > everywhere. Kernel threads are not allowed to access __user pointers
> > anyway.
> > 
> > Signed-off-by: Arnd Bergmann 
> > ---
> >  arch/sparc/include/asm/processor_64.h   |  4 
> >  arch/sparc/include/asm/switch_to_64.h   |  4 +---
> >  arch/sparc/include/asm/thread_info_64.h |  4 +---
> >  arch/sparc/include/asm/uaccess_64.h | 20 +---
> >  arch/sparc/kernel/process_64.c  | 12 
> >  arch/sparc/kernel/traps_64.c|  2 --
> >  arch/sparc/lib/NGmemcpy.S   |  3 +--
> >  arch/sparc/mm/init_64.c |  7 ---
> >  8 files changed, 8 insertions(+), 48 deletions(-)
> 
> I think you somehow missed the Kconfig change, and also the related
> sparc32 change which continue to have set_fs() after this patch.
I now notice the sparc32 bits are in the last patch.
To avoid breaking bisect-ability on sparc64 I think you need to merge
the sparc32 changes with this patch, unless the sparc64 changes can
coexist with CONFIG_SET_FS continue to be set.

Sam

Re: [PATCH/RFC] mm: add and use batched version of __tlb_remove_table()

[Sam Ravnborg]

Hi Nikita,

How about adding the following to tlb.h:

#ifndef __tlb_remove_tables
static void __tlb_remove_tables(...)
{

}
#endif


And then the few archs that want to override __tlb_remove_tables
needs to do a
#define __tlb_remove_tables __tlb_remove_tables
static void __tlb_remove_tables(...)
{
...
}

In this way the archs that uses the default implementation needs not do
anything.
A few functions already uses this pattern in tlb.h - see for example 
tlb_start_vma
io.h is another file where you can see the same pattern.

Sam

Re: [PATCH] agp: define proper stubs for empty helpers

[Sam Ravnborg]

On Mon, Sep 20, 2021 at 02:17:19PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> The empty unmap_page_from_agp() macro causes a warning when
> building with 'make W=1' on a couple of architectures:
> 
> drivers/char/agp/generic.c: In function 'agp_generic_destroy_page':
> drivers/char/agp/generic.c:1265:28: error: suggest braces around empty body 
> in an 'if' statement [-Werror=empty-body]
>  1265 |   unmap_page_from_agp(page);
> 
> Change the definitions to a 'do { } while (0)' construct to
> make these more reliable.
> 
> Signed-off-by: Arnd Bergmann 

Looks good.
Acked-by: Sam Ravnborg 

Sam

Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option

[Sam Ravnborg]

Hi Krzysztof,
On Mon, Nov 30, 2020 at 10:25:01PM +0200, Krzysztof Kozlowski wrote:
> On Mon, Nov 30, 2020 at 08:11:33PM +0100, Sam Ravnborg wrote:
> > On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> > > Since the removal of generic_bl driver from the source tree in commit
> > > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > > therefore subject to clean-up from all configuration files.
> > > 
> > > This series introduces patches to address this removal, separated by
> > > architectures in the kernel tree.
> > > 
> > > Andrey Zhizhikin (5):
> > >   ARM: configs: drop unused BACKLIGHT_GENERIC option
> > >   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> > >   MIPS: configs: drop unused BACKLIGHT_GENERIC option
> > >   parisc: configs: drop unused BACKLIGHT_GENERIC option
> > >   powerpc/configs: drop unused BACKLIGHT_GENERIC option
> > 
> > For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
> > savedefconfig / cp defconfig ... run now and then - this will remove
> > all such symbols.
> 
> savedefconfig can be tricky because of risk of loosing options:
> 1. it will remove options which became the default or became selected,
> 2. later when the default is changed or selecting option is removed, the
>first option from #1 will not be brought back.
> 
> This was already for example with DEBUG_FS and the conclusion that time
> was - do not run savedefconfig automatically.
> 
> Therefore if some symbol(s) can be safely removed, patch is welcomed.

Thanks for letting me know, I have missed that discussion and
was obviously not aware.
I already acked'ed the patches and hope the soc people will pick them
up.

Sam

Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option

[Sam Ravnborg]

On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> Since the removal of generic_bl driver from the source tree in commit
> 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") BACKLIGHT_GENERIC config option became obsolete as well and
> therefore subject to clean-up from all configuration files.
> 
> This series introduces patches to address this removal, separated by
> architectures in the kernel tree.
> 
> Andrey Zhizhikin (5):
>   ARM: configs: drop unused BACKLIGHT_GENERIC option
>   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
>   MIPS: configs: drop unused BACKLIGHT_GENERIC option
>   parisc: configs: drop unused BACKLIGHT_GENERIC option
>   powerpc/configs: drop unused BACKLIGHT_GENERIC option

For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
savedefconfig / cp defconfig ... run now and then - this will remove
all such symbols.

If the patches goes in like they are submitted then:
Acked-by: Sam Ravnborg 

> 
>  arch/arm/configs/at91_dt_defconfig  | 1 -
>  arch/arm/configs/cm_x300_defconfig  | 1 -
>  arch/arm/configs/colibri_pxa300_defconfig   | 1 -
>  arch/arm/configs/jornada720_defconfig   | 1 -
>  arch/arm/configs/magician_defconfig | 1 -
>  arch/arm/configs/mini2440_defconfig | 1 -
>  arch/arm/configs/omap2plus_defconfig| 1 -
>  arch/arm/configs/pxa3xx_defconfig   | 1 -
>  arch/arm/configs/qcom_defconfig | 1 -
>  arch/arm/configs/sama5_defconfig| 1 -
>  arch/arm/configs/sunxi_defconfig| 1 -
>  arch/arm/configs/tegra_defconfig| 1 -
>  arch/arm/configs/u8500_defconfig| 1 -
>  arch/arm64/configs/defconfig| 1 -
>  arch/mips/configs/gcw0_defconfig| 1 -
>  arch/mips/configs/gpr_defconfig | 1 -
>  arch/mips/configs/lemote2f_defconfig| 1 -
>  arch/mips/configs/loongson3_defconfig   | 1 -
>  arch/mips/configs/mtx1_defconfig| 1 -
>  arch/mips/configs/rs90_defconfig| 1 -
>  arch/parisc/configs/generic-64bit_defconfig | 1 -
>  arch/powerpc/configs/powernv_defconfig  | 1 -
>  22 files changed, 22 deletions(-)
> 
> 
> base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
> prerequisite-patch-id: bfd382cf1dc021d20204f10ea9403319c1c32b12
> prerequisite-patch-id: 5397c0c8648bb3e0b830207ea867138c11c6e644
> prerequisite-patch-id: a3c284dff5fe6d02828918a886db6a8ed3197e20
> -- 
> 2.17.1

Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link

[Sam Ravnborg]

Hi Masahiro.

On Sun, May 24, 2020 at 12:12:35AM +0900, Masahiro Yamada wrote:
> Hi Nicholas,
> (+CC: Sam Ravnborg)
> 
> 
> On Sat, May 23, 2020 at 7:06 PM Nicholas Piggin  wrote:
> >
> > Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am:
> > > + Michael, and PPC ML.
> > >
> > > They may know something about the reason of failure.
> >
> > Because the linker can't put branch stubs within object code sections,
> > so when you incrementally link them too large, the linker can't resolve
> > branches into other object files.
> 
> 
> Ah, you are right.
> 
> So, this is a problem not only for PPC
> but also for ARM (both 32 and 64 bit), etc.
> 
> ARM needs to insert a veneer to jump far.
> 
> Prior to thin archive, we could not compile
> ARCH=arm allyesconfig because
> drivers/built-in.o was too large.
> 
> This patch gets us back to the too large
> incremental object situation.
> 
> With my quick compile-testing,
> ARCH=arm allyesconfig
> and ARCH=arm64 allyesconfig are broken.
> 
> 
> > This is why we added incremental linking in the first place. I suppose
> > it could be made conditional for platforms that can use this
> > optimization.
> >
> > What'd be really nice is if we could somehow build and link kallsyms
> > without relinking everything twice, and if we could do section mismatch
> > analysis without making that vmlinux.o as well. I had a few ideas but
> > not enough time to do much work on it.
> 
> 
> Right, kallsyms links 3 times. (not twice)
> 
> 
> Hmm, I think Sami's main motivation is Clang LTO.
> 
> LTO is very time-consuming.
> So, the android common kernel implements Clang LTO
> in the pre modpost stage:
> 
> 
> 1) LTO against vmlinux.o
> 
> 2) modpost against vmlinux.o
> 
> 3) Link vmlinux.o + kallsyms into vmlinux
>(this requires linking 3 times)

We have kallsyms we had to link three times because the linking
increased the object a little in size so symbols did not match.
The last time was added more or less only to check that we did
have stable symbol addresses.

All this predates LTO stuff which we only introduced later.

The reason for doing modpost on vmlinux.o was that we had cases
where everything in drivers/ was fine but there was section mismatch
references from arch/* to drivers/*
This is back when there were much more drivers in arch/ than what we
have today.
And back then we also had much more to check ad we had cPU hotplug
that could really cause section mismatches - this is no longer the case
which is a good thing.



...
> 
> The following two commits.
> I did not fully understand the background, though.
> 
> I CC'ed Sam in case he may add some comments.
> 
> commit 85bd2fddd68e757da8e1af98f857f61a3c9ce647
> Author: Sam Ravnborg 
> Date:   Mon Feb 26 15:33:52 2007 +0100
> 
> kbuild: fix section mismatch check for vmlinux
> 
> vmlinux does not contain relocation entries which is
> used by the section mismatch checks.
> Reported by: Atsushi Nemoto 
> 
> Use the individual objects as inputs to overcome
> this limitation.
> In modpost check the .o files and skip non-ELF files.
> 
> Signed-off-by: Sam Ravnborg 


So we checked vmlinx - but vmlinx did have too much stripped away.
so in reality nothing was checked.
To allow the warnings to be as precise as possible move the checks
out to the indovidual .o files.
Sometimes the names was mangled a little so if warnigns was only
reported on vmlinx level in could be difficult to track down the
offender.
This would then also do the check on .o files that had all the
relocation symbols rtequired.

> 
> commit 741f98fe298a73c9d47ed53703c1279a29718581
> Author: Sam Ravnborg 
> Date:   Tue Jul 17 10:54:06 2007 +0200
> 
> kbuild: do section mismatch check on full vmlinux
> 
> Previously we did do the check on the .o files used to link
> vmlinux but that failed to find questionable references across
> the .o files.
> Create a dedicated vmlinux.o file used only for section mismatch checks
> that uses the defualt linker script so section does not get renamed.
> 
> The vmlinux.o may later be used as part of the the final link of vmlinux
> but for now it is used fo section mismatch only.
> For a defconfig build this is instant but for an allyesconfig this
> add two minutes to a full build (that anyways takes ~2 hours).
> 
> Signed-off-by: Sam Ravnborg 

But when we introduced check of the individual .o fiules we missed when
the references spanned outside the .o files as explained previously.
So included a link of vmlinx.o that did NOT drop the relocations
so we could use it to check for the remaining

Re: [PATCH v2 5/6] arch: simplify several early memory allocations

[Sam Ravnborg]

On Mon, Dec 03, 2018 at 06:49:21PM +0200, Mike Rapoport wrote:
> On Mon, Dec 03, 2018 at 05:29:08PM +0100, Sam Ravnborg wrote:
> > Hi Mike.
> > 
> > > index c37955d..2a17665 100644
> > > --- a/arch/sparc/kernel/prom_64.c
> > > +++ b/arch/sparc/kernel/prom_64.c
> > > @@ -34,16 +34,13 @@
> > >  
> > >  void * __init prom_early_alloc(unsigned long size)
> > >  {
> > > - unsigned long paddr = memblock_phys_alloc(size, SMP_CACHE_BYTES);
> > > - void *ret;
> > > + void *ret = memblock_alloc(size, SMP_CACHE_BYTES);
> > >  
> > > - if (!paddr) {
> > > + if (!ret) {
> > >   prom_printf("prom_early_alloc(%lu) failed\n", size);
> > >   prom_halt();
> > >   }
> > >  
> > > - ret = __va(paddr);
> > > - memset(ret, 0, size);
> > >   prom_early_allocated += size;
> > >  
> > >   return ret;
> > 
> > memblock_alloc() calls memblock_alloc_try_nid().
> > And if allocation fails then memblock_alloc_try_nid() calls panic().
> > So will we ever hit the prom_halt() code?
> 
> memblock_phys_alloc_try_nid() also calls panic if an allocation fails. So
> in either case we never reach prom_halt() code.

So we have code here we never reach - not nice.
If the idea is to avoid relying on the panic inside memblock_alloc() then
maybe replace it with a variant that do not call panic?
To make it clear what happens.

Sam

Re: [PATCH v2 5/6] arch: simplify several early memory allocations

[Sam Ravnborg]

Hi Mike.

> index c37955d..2a17665 100644
> --- a/arch/sparc/kernel/prom_64.c
> +++ b/arch/sparc/kernel/prom_64.c
> @@ -34,16 +34,13 @@
>  
>  void * __init prom_early_alloc(unsigned long size)
>  {
> - unsigned long paddr = memblock_phys_alloc(size, SMP_CACHE_BYTES);
> - void *ret;
> + void *ret = memblock_alloc(size, SMP_CACHE_BYTES);
>  
> - if (!paddr) {
> + if (!ret) {
>   prom_printf("prom_early_alloc(%lu) failed\n", size);
>   prom_halt();
>   }
>  
> - ret = __va(paddr);
> - memset(ret, 0, size);
>   prom_early_allocated += size;
>  
>   return ret;

memblock_alloc() calls memblock_alloc_try_nid().
And if allocation fails then memblock_alloc_try_nid() calls panic().
So will we ever hit the prom_halt() code?

Do we have a panic() implementation that actually returns?


> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> index 3c8aac2..52884f4 100644
> --- a/arch/sparc/mm/init_64.c
> +++ b/arch/sparc/mm/init_64.c
> @@ -1089,16 +1089,13 @@ static void __init allocate_node_data(int nid)
>   struct pglist_data *p;
>   unsigned long start_pfn, end_pfn;
>  #ifdef CONFIG_NEED_MULTIPLE_NODES
> - unsigned long paddr;
>  
> - paddr = memblock_phys_alloc_try_nid(sizeof(struct pglist_data),
> - SMP_CACHE_BYTES, nid);
> - if (!paddr) {
> + NODE_DATA(nid) = memblock_alloc_node(sizeof(struct pglist_data),
> +  SMP_CACHE_BYTES, nid);
> + if (!NODE_DATA(nid)) {
>   prom_printf("Cannot allocate pglist_data for nid[%d]\n", nid);
>   prom_halt();
>   }
> - NODE_DATA(nid) = __va(paddr);
> - memset(NODE_DATA(nid), 0, sizeof(struct pglist_data));
>  
>   NODE_DATA(nid)->node_id = nid;
>  #endif

Same here.

I did not look at the other cases.

Sam

Re: [PATCH v2 3/6] sh: prefer memblock APIs returning virtual address

[Sam Ravnborg]

Hi Mike.

On Mon, Dec 03, 2018 at 05:47:12PM +0200, Mike Rapoport wrote:
> Rather than use the memblock_alloc_base that returns a physical address and
> then convert this address to the virtual one, use appropriate memblock
> function that returns a virtual address.
> 
> There is a small functional change in the allocation of then NODE_DATA().
> Instead of panicing if the local allocation failed, the non-local
> allocation attempt will be made.
> 
> Signed-off-by: Mike Rapoport 
> ---
>  arch/sh/mm/init.c | 18 +-
>  arch/sh/mm/numa.c |  5 ++---
>  2 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index c8c13c77..3576b5f 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -192,24 +192,16 @@ void __init page_table_range_init(unsigned long start, 
> unsigned long end,
>  void __init allocate_pgdat(unsigned int nid)
>  {
>   unsigned long start_pfn, end_pfn;
> -#ifdef CONFIG_NEED_MULTIPLE_NODES
> - unsigned long phys;
> -#endif
>  
>   get_pfn_range_for_nid(nid, _pfn, _pfn);
>  
>  #ifdef CONFIG_NEED_MULTIPLE_NODES
> - phys = __memblock_alloc_base(sizeof(struct pglist_data),
> - SMP_CACHE_BYTES, end_pfn << PAGE_SHIFT);
> - /* Retry with all of system memory */
> - if (!phys)
> - phys = __memblock_alloc_base(sizeof(struct pglist_data),
> - SMP_CACHE_BYTES, 
> memblock_end_of_DRAM());
> - if (!phys)
> + NODE_DATA(nid) = memblock_alloc_try_nid_nopanic(
> + sizeof(struct pglist_data),
> + SMP_CACHE_BYTES, MEMBLOCK_LOW_LIMIT,
> + MEMBLOCK_ALLOC_ACCESSIBLE, nid);
> + if (!NODE_DATA(nid))
>   panic("Can't allocate pgdat for node %d\n", nid);
> -
> - NODE_DATA(nid) = __va(phys);
> - memset(NODE_DATA(nid), 0, sizeof(struct pglist_data));
The new code will always assign NODE_DATA(nid), where the old
code only assigned NODE_DATA(nid) in the good case.
I dunno if this is an issue, just noticed the difference and
wanted to point it out.

Sam

Re: [PATCH 00/25] Change tty_port(standard)_install's return type

[Sam Ravnborg]

Hi Jaejoong.

> Change return type for tty functions. Patch No.01
>   tty: Change return type to void
Adding this patch first will generate a lot of warnings
until all users are updated.
It is usual practice to prepare all users
and then apply the infrastructure changes as the
last patch.
Then people will not see a lot of warnings when
they build something in the middle,
and I guess current stack set may also generate
a few mails from the 0-day build infrastructure.


>   isdn: i4l: isdn_tty: Change return type to void

And a nitpick on the patch description.
This patch do not change any return type, but
it ignore the return value og tty_part_install().
Same goes for all ramaining patches.

Sam

Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

[Sam Ravnborg]

Hi Masahiro.

On Sat, Feb 17, 2018 at 03:38:28AM +0900, Masahiro Yamada wrote:
> I brushed up the implementation in this version.
> 
> In the previous RFC, CC_HAS_ was described by using 'option shell=',
> like this:
> 
> config CC_HAS_STACKPROTECTOR
> bool
> option shell="$CC -Werror -fstack-protector -c -x c /dev/null"
> 
> After I thought a bit more, the following syntax is more grammatical,
> and flexible.
> 
> config CC_HAS_STACKPROTECTOR
> bool
> default $(shell $CC -Werror -fstack-protector -c -x c /dev/null)

Looks good - but maybe we should go one step further.

So we in the syntax explicit handles:
- shell commands
- other commands, defined as strings
- environment variables
- config variables

Each case is explicit - so the reader is not confused what is used when.

$(shell foo)- output of the shell command foo. Uses $SHELL as the shell.
  May include optional paramters.
  foo may be a config variable referenced using ${} or a config 
variable prefixed with $
Example:

config BUILD_DIR
string
default $(shell cd ${objtree}; pwd)

$(call bar) - output of the bar command that may take optional parameters.
  bar may be a text string, a config variable or an environment 
variable
   The definition of bar may reference the parameters using 
$(1), $(2)
   In this context a config variable needs to be prefixed with $

Example:

config reverse
string
default $(2) $(1)

config NEW_ORDER
string
$(call $reverse, A, B)  # Will assign REVERSE the value 
"B A"


Example2:

config CC_OPTION
string
default $(shell ${srctree}/scripts/cc-option ${CC} $(1) 
$(2))

config CC_OPTIMIZE
string
$(call $CC_OPTION, -Oz, -Os)


${FOO}  - environment variable

The above is inspired by how make implement similar functionality.

I'm not happy that we in one context can reference CONFIG variables
directly, but inside the $(call ...) and $(shell ...) needs the $ prefix.
But I could not come up with something un-ambigious where this could be avoided.

The above proposal include the functionality of the macro stuff proposed in 
this patch-set.
But with a simpler syntax and we keep all the other kconfig logic (depends on 
etc) - so
users will not be limited in their creativity.

> Current limitations:
> 
> Dependency on outside scripts.
> Inter-option dependency:
> Functions are evaluated statically:

Same limitations exists with the syntax suggested above.

Sam

Re: [PATCH v5 1/8] arch: enable relative relocations for arm64, power, x86, s390 and x86

[Sam Ravnborg]

Hi Ard.

On Mon, Dec 25, 2017 at 08:54:33PM +, Ard Biesheuvel wrote:
> Before updating certain subsystems to use place relative 32-bit
> relocations in special sections, to save space  and reduce the
> number of absolute relocations that need to be processed at runtime
> by relocatable kernels, introduce the Kconfig symbol and define it
> for some architectures that should be able to support and benefit
> from it.
> 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: x...@kernel.org
> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/Kconfig| 10 ++
>  arch/arm64/Kconfig  |  1 +

>  arch/arm64/kernel/vmlinux.lds.S |  2 +-
The change to arch/arm64/kernel/vmlinux.lds.S is
not justified in the changelog.
Did you add it by mistake?

Sam

Re: [v5 09/15] sparc64: optimized struct page zeroing

[Sam Ravnborg]

Hi Pavel.

On Thu, Aug 03, 2017 at 05:23:47PM -0400, Pavel Tatashin wrote:
> Add an optimized mm_zero_struct_page(), so struct page's are zeroed without
> calling memset(). We do eight regular stores, thus avoid cost of membar.

The commit message does no longer reflect the implementation,
and should be updated.

> 
> Signed-off-by: Pavel Tatashin 
> Reviewed-by: Steven Sistare 
> Reviewed-by: Daniel Jordan 
> Reviewed-by: Bob Picco 
> ---
>  arch/sparc/include/asm/pgtable_64.h | 32 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/sparc/include/asm/pgtable_64.h 
> b/arch/sparc/include/asm/pgtable_64.h
> index 6fbd931f0570..be47537e84c5 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -230,6 +230,38 @@ extern unsigned long _PAGE_ALL_SZ_BITS;
>  extern struct page *mem_map_zero;
>  #define ZERO_PAGE(vaddr) (mem_map_zero)
>  
> +/* This macro must be updated when the size of struct page grows above 80
> + * or reduces below 64.
> + * The idea that compiler optimizes out switch() statement, and only
> + * leaves clrx instructions or memset() call.
> + */
> +#define  mm_zero_struct_page(pp) do {
> \
> + unsigned long *_pp = (void *)(pp);  \
> + \
> + /* Check that struct page is 8-byte aligned */  \
> + BUILD_BUG_ON(sizeof(struct page) & 7);  \
Would also be good to catch if sizeof > 80 so we do not silently
migrate to the suboptimal version (silent at build time).
Can you at build time catch if size is no any of: 64, 72, 80
and simplify the below a little?

Sam

Re: [PATCH 2/5] kbuild: allow archs to select build for link dead code/data elimination

[Sam Ravnborg]

On Mon, Aug 08, 2016 at 01:29:03PM +1000, Nicholas Piggin wrote:
> On Sat, 6 Aug 2016 22:14:23 +0200
> Sam Ravnborg <s...@ravnborg.org> wrote:
> 
> > On Fri, Aug 05, 2016 at 10:12:00PM +1000, Nicholas Piggin wrote:
> > > Introduce LINKER_DCE option for architectures to select if they want
> > > to build with -ffunction-sections, -fdata-sections, and link with
> > > --gc-sections.  
> > 
> > Can you please try to come up with a less cryptic name.
> > "DCE" may make sense for you today.
> > Bot the naive reader will benefit from the longer and
> > more explcit form.
> 
> Yes that's a good idea. The name sucks.
> 
> We don't seem to consistently have a prefix for build configuration
> options, but we could start? KBUILD_LD_DEAD_CODE_DATA_ELIMINATION?

For cc related options we sort of have: KCONFIG_CC IIRC.
So for LD related options we could use the shorter CONFIG_LD_ prefix.

Sam

Re: [PATCH 1/5] kbuild: allow architectures to use thin archives instead of ld -r

[Sam Ravnborg]

On Mon, Aug 08, 2016 at 01:19:41PM +1000, Nicholas Piggin wrote:
> On Sun, 7 Aug 2016 16:40:54 +0200
> Sam Ravnborg <s...@ravnborg.org> wrote:
> 
> > On Sun, Aug 07, 2016 at 11:49:46AM +1000, Stephen Rothwell wrote:
> > > Hi Sam,
> > > 
> > > On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg <s...@ravnborg.org> wrote: 
> > >  
> > > >
> > > > Did you by any chance evalue the use of INPUT in linker files.
> > > > Stephen back then (again based on proposal from Alan Modra),
> > > > also made an implementation using INPUT.  
> > > 
> > > The problem with that idea was that (at least for some versions of
> > > binutils in use at the time) we hit a static limit to the number of
> > > object files and ld just stopped at that point. :-(  
> > 
> > The ld bug was caused by opening too many linked definitions files.
> > We can workaround this by expanding the files.
> > I gave this a quick spin - see below.
> > 
> > Note - I have no idea if using thin archived or this method is better.
> > But it seems just wrong to me that we convert to thin archives when
> > we really do not need to do so.
> > 
> > Note - this was a quick spin. It build fine here and thats it.
> 
> Is there a reason to prefer using linker scripts rather than thin
> archives? I thought the former was possibly a bit less robust, and
> the latter a smaller change for scripts and toolchain in terms
> of "almost behaving like an object file".

The only valid reason I can come up with is that it is simpler
to build a list of .o files than it is to generate a number
of thin archives.

I persuaded "linker scripts" mainly because I had the patch floating
and I was triggered by the powerpc discussions to resurface it again.

> I don't have a strong preference although do have a couple of
> (out of tree) scripts that expect objdump to work on built-in.o

You are likely not alone here.
Unless someone else comes up with any good reason lets stay with
thin archives.

Sam

Re: [PATCH 1/5] kbuild: allow architectures to use thin archives instead of ld -r

[Sam Ravnborg]

On Sun, Aug 07, 2016 at 11:49:46AM +1000, Stephen Rothwell wrote:
> Hi Sam,
> 
> On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg <s...@ravnborg.org> wrote:
> >
> > Did you by any chance evalue the use of INPUT in linker files.
> > Stephen back then (again based on proposal from Alan Modra),
> > also made an implementation using INPUT.
> 
> The problem with that idea was that (at least for some versions of
> binutils in use at the time) we hit a static limit to the number of
> object files and ld just stopped at that point. :-(

The ld bug was caused by opening too many linked definitions files.
We can workaround this by expanding the files.
I gave this a quick spin - see below.

Note - I have no idea if using thin archived or this method is better.
But it seems just wrong to me that we convert to thin archives when
we really do not need to do so.

Note - this was a quick spin. It build fine here and thats it.

Sam

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 11602e5..37b8bc9 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -360,10 +360,16 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;
 ifdef builtin-target
 quiet_cmd_link_o_target = LD  $@
 # If the list of objects to link is empty, just create an empty built-in.o
-cmd_link_o_target = $(if $(strip $(obj-y)),\
- $(LD) $(ld_flags) -r -o $@ $(filter $(obj-y), $^) \
- $(cmd_secanalysis),\
- rm -f $@; $(AR) rcs$(KBUILD_ARFLAGS) $@)
+cmd_link_o_target =\
+$(if $(filter $(obj-y), $^),   \
+   echo $(foreach file, $(filter $(obj-y), $^),   \
+   $(if $(filter %/built-in.o, $(file)),  \
+   $(shell cat $(file)),  \
+   $(file)\
+   )  \
+   )  \
+, echo ""  \
+) > $@
 
 $(builtin-target): $(obj-y) FORCE
$(call if_changed,link_o_target)
@@ -414,10 +420,10 @@ $($(subst $(obj)/,,$(@:.o=-y)))   \
 $($(subst $(obj)/,,$(@:.o=-m, $^)
 
 quiet_cmd_link_multi-y = LD  $@
-cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps) 
$(cmd_secanalysis)
+cmd_link_multi-y = echo INPUT\($(link_multi_deps)\) > $@
 
 quiet_cmd_link_multi-m = LD [M]  $@
-cmd_link_multi-m = $(cmd_link_multi-y)
+cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
 
 $(multi-used-y): FORCE
$(call if_changed,link_multi-y)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 4f727eb..323c13a 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -41,8 +41,8 @@ info()
 # ${1} output file
 modpost_link()
 {
-   ${LD} ${LDFLAGS} -r -o ${1} ${KBUILD_VMLINUX_INIT}   \
-   --start-group ${KBUILD_VMLINUX_MAIN} --end-group
+   ${LD} ${LDFLAGS} -r -o ${1} ${vmlinux_init}\
+   --start-group ${vmlinux_main} --end-group
 }
 
 # Link of vmlinux
@@ -54,13 +54,13 @@ vmlinux_link()
 
if [ "${SRCARCH}" != "um" ]; then
${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2}  \
-   -T ${lds} ${KBUILD_VMLINUX_INIT} \
-   --start-group ${KBUILD_VMLINUX_MAIN} --end-group ${1}
+   -T ${lds} ${vmlinux_init}\
+   --start-group ${vmlinux_main} --end-group ${1}
else
${CC} ${CFLAGS_vmlinux} -o ${2}  \
-   -Wl,-T,${lds} ${KBUILD_VMLINUX_INIT} \
+   -Wl,-T,${lds} ${vmlinux_init}\
-Wl,--start-group\
-${KBUILD_VMLINUX_MAIN}  \
+${vmlinux_main} \
-Wl,--end-group  \
-lutil -lrt -lpthread ${1}
rm -f linux
@@ -162,6 +162,23 @@ case "${KCONFIG_CONFIG}" in
. "./${KCONFIG_CONFIG}"
 esac
 
+# Expand built-in.o file as they just list .o files
+for f in ${KBUILD_VMLINUX_INIT}; do
+   if [ $(basename $f) = built-in.o ]; then
+   vmlinux_init="${vmlinux_init} $(cat $f)"
+   else
+   vmlinux_init="${vmlinux_init} $f"
+   fi
+done
+
+for f in ${KBUILD_VMLINUX_MAIN}; do
+   if [ $(basename $f) = built-in.o ]; then
+   vmlinux_main="${vmlinux_main} $(cat $f)"
+   else
+   vmlinux_main="${vmlinux_main} $f"
+   fi
+done
+
 #link vmlinux.o
 info LD vmlinux.o
 modpost_link vmlinux.o

Re: [PATCH 1/5] kbuild: allow architectures to use thin archives instead of ld -r

[Sam Ravnborg]

Hi Nicholas.

On Fri, Aug 05, 2016 at 10:11:59PM +1000, Nicholas Piggin wrote:
> From: Stephen Rothwell 
> 
> ld -r is an incremental link used to create built-in.o files in build
> subdirectories. It produces relocatable object files containing all
> its input files, and these are are then pulled together and relocated
> in the final link. Aside from the bloat, this constrains the final
> link relocations, which has bitten large powerpc builds with
> unresolvable relocations in the final link.
> 
> Alan Modra has recommended the kernel use thin archives for linking.
> This is an alternative and means that the linker has more information
> available to it when it links the kernel.
> 
> This patch enables a config option architectures can select,
If we want to do this, then I suggest to make the logic reverse.
Architectures that for some reasons cannot use this should
have the possibility to avoid it. But let it be enabled by default.

> which
> causes all built-in.o files to be built as thin archives. built-in.o
> files in subdirectories do not get symbol table or index attached,
> which improves speed and size. The final link pass creates a
> built-in.o archive in the root output directory which includes the
> symbol table and index. The linker then uses takes this file to link.
> 
> The --whole-archive linker option is required, because the linker now
> has visibility to every individual object file, and it will otherwise
> just completely avoid including those without external references
> (consider a file with EXPORT_SYMBOL or initcall or hardware exceptions
> as its only entry points). The traditional built works "by luck" as
> built-in.o files are large enough that they're going to get external
> references. However this optimisation is unpredictable for the kernel
> (due to above external references), ineffective at culling unused, and
> costly because the .o files have to be searched for references.
> Superior alternatives for link-time culling should be used instead.
> 
> Build characteristics for inclink vs thinarc, on a small powerpc64le
> pseries VM with a modest .config:
> 
>   inclink   thinarc
> sizes
> vmlinux15 618 68015 625 028
> sum of all built-in.o  56 091 808 1 054 334
> sum excluding root built-in.o   151 430
> 
> find -name built-in.o | xargs rm ; time make vmlinux
> real  22.772s   21.143s
> user  13.280s   13.430s
> sys4.310s2.750s
> 
> - Final kernel pulled in only about 6K more, which shows how
>   ineffective the object file culling is.
> - Build performance looks improved due to less pagecache activity.
>   On IO constrained systems it could be a bigger win.
> - Build size saving is significant.

Good to see this old proposal picked up again!

Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
See below for an updated simple patch on top of mainline.

Build statistics for "make defconfig" on my i7 box:

find -name built-in.o; xargs rm; time make -j16 vmlinux
standardsinglelink  delta
real0m6.368s0m7.040s+672ms
user0m15.577s   0m14.960s   -617ms
sys 0m7.601s0m6.226s-1375ms

vmlinux size:   standardsinglelink  delta
text10.250.675  10.250.675  0
data4.369.632   4.374.816   +5184
bss 1.110.016   1.110.016   0

I had expected to see improvements in build time - but
we serialize the heavy link phase, so it is actually slower.

I did not investigate why data section got larger,
but I think you already touch the reasons.

The patch does not change how we link modules.

Please consider if this approach is better / worse than
using archieves.

Note that this patch remove the possibility to run section
mismatch anylysis on a per-directory basis.

Sam

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 11602e5..954e7cb 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -360,10 +360,9 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;
 ifdef builtin-target
 quiet_cmd_link_o_target = LD  $@
 # If the list of objects to link is empty, just create an empty built-in.o
-cmd_link_o_target = $(if $(strip $(obj-y)),\
- $(LD) $(ld_flags) -r -o $@ $(filter $(obj-y), $^) \
- $(cmd_secanalysis),\
- rm -f $@; $(AR) rcs$(KBUILD_ARFLAGS) $@)
+cmd_link_o_target = $(if $(filter $(obj-y), $^),   \
+echo INPUT\($(filter $(obj-y), $^)\) > $@, \
+echo "/* empty */" > $@)
 
 $(builtin-target): $(obj-y) FORCE
$(call if_changed,link_o_target)
@@ 

Re: [PATCH 2/5] kbuild: allow archs to select build for link dead code/data elimination

[Sam Ravnborg]

On Fri, Aug 05, 2016 at 10:12:00PM +1000, Nicholas Piggin wrote:
> Introduce LINKER_DCE option for architectures to select if they want
> to build with -ffunction-sections, -fdata-sections, and link with
> --gc-sections.

Can you please try to come up with a less cryptic name.
"DCE" may make sense for you today.
Bot the naive reader will benefit from the longer and
more explcit form.


 It requires some work (documented) to ensure all
> unreferenced entrypoints are live, and requires toolchain and
> build verification, so it is made a per-arch option for now.
> 
> On a random powerpc64le build, this yelds a significant size saving,
> it boots and runs fine, but there is a lot I haven't tested as yet,
> so these savings may be reduced if there are bugs in the link.
> 
> text  databssdec   filename
> 11169741   11807441923176 14273661   vmlinux
> 10445269   10041271919707 13369103   vmlinux.dce
> 
> ~700K text, ~170K data, 6% removed from kernel image size.
> 
> Signed-off-by: Nicholas Piggin 
> ---
>  Makefile  | 10 
>  arch/Kconfig  | 13 ++
>  include/asm-generic/vmlinux.lds.h | 52 
> ++-
>  include/linux/compiler.h  | 18 ++
>  include/linux/export.h| 30 +++---
>  include/linux/init.h  | 38 ++--
>  init/Makefile |  2 ++
>  7 files changed, 100 insertions(+), 63 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index b409076..d5ef31a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -618,6 +618,11 @@ include arch/$(SRCARCH)/Makefile
>  
>  KBUILD_CFLAGS+= $(call cc-option,-fno-delete-null-pointer-checks,)
>  
> +ifdef CONFIG_LINKER_DCE
> +KBUILD_CFLAGS+= $(call cc-option,-ffunction-sections,)
> +KBUILD_CFLAGS+= $(call cc-option,-fdata-sections,)
> +endif
> +
>  ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
>  KBUILD_CFLAGS+= -Os $(call cc-disable-warning,maybe-uninitialized,)
>  else
> @@ -819,6 +824,11 @@ LDFLAGS_BUILD_ID = $(patsubst -Wl$(comma)%,%,\
>  KBUILD_LDFLAGS_MODULE += $(LDFLAGS_BUILD_ID)
>  LDFLAGS_vmlinux += $(LDFLAGS_BUILD_ID)
>  
> +ifdef CONFIG_LINKER_DCE
> +# LDFLAGS_MODULE += $(call ld-option, --gc-sections,)
> +LDFLAGS_vmlinux  += $(call ld-option, --gc-sections,)
> +endif
Something you missed to clean up

Sam

Re: [PATCH 3/5] kbuild: add arch specific post-module-link pass

[Sam Ravnborg]

On Fri, Aug 05, 2016 at 10:12:01PM +1000, Nicholas Piggin wrote:
> Add an option for architectures to pass over modules after they are
> linked. powerpc will use this to fix up alternate instruction patch
> relocations.
> 
> Signed-off-by: Nicholas Piggin 
> ---
>  Documentation/kbuild/makefiles.txt | 6 ++
>  Makefile   | 1 +
>  scripts/Makefile.modpost   | 8 
>  3 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/kbuild/makefiles.txt 
> b/Documentation/kbuild/makefiles.txt
> index 13f888a..f6c065b 100644
> --- a/Documentation/kbuild/makefiles.txt
> +++ b/Documentation/kbuild/makefiles.txt
> @@ -952,6 +952,12 @@ When kbuild executes, the following steps are followed 
> (roughly):
>   $(KBUILD_ARFLAGS) set by the top level Makefile to "D" (deterministic
>   mode) if this option is supported by $(AR).
>  
> +KBUILD_MODPOST_TOOL   Arch-specific command to run after module link
> +
> +$(KBUILD_MODPOST_TOOL) is used to add an arch-specific pass over
> +modules after their final link. E.g., powerpc uses this to adjust
> +relative branches of "alternate code patching" sections.
> +

This needs documentation in kbuild.txt, where there is a
nearly full lst of KBUILD_ variables.

Sam

Re: Build regressions/improvements in v3.16-rc1

[Sam Ravnborg]

 
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_add' 
  [-Werror=implicit-function-declaration]:  = 176:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_add_negative' 
  [-Werror=implicit-function-declaration]:  = 211:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_add_return' 
  [-Werror=implicit-function-declaration]:  = 218:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_dec' 
  [-Werror=implicit-function-declaration]:  = 169:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_dec_and_test' 
  [-Werror=implicit-function-declaration]:  = 197:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_dec_return' 
  [-Werror=implicit-function-declaration]:  = 239:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_inc' 
  [-Werror=implicit-function-declaration]:  = 162:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_inc_and_test' 
  [-Werror=implicit-function-declaration]:  = 204:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_inc_return' 
  [-Werror=implicit-function-declaration]:  = 232:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_set' 
  [-Werror=implicit-function-declaration]:  = 155:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_sub' 
  [-Werror=implicit-function-declaration]:  = 183:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_sub_and_test' 
  [-Werror=implicit-function-declaration]:  = 190:2
+ /scratch/kisskb/src/include/asm-generic/atomic-long.h: error: implicit 
  declaration of function 'atomic_sub_return' 
  [-Werror=implicit-function-declaration]:  = 225:2
+ /scratch/kisskb/src/include/linux/atomic.h: error: implicit declaration 
  of function '__atomic_add_unless' [-Werror=implicit-function-declaration]:  
  = 53:2
+ /scratch/kisskb/src/include/linux/atomic.h: error: implicit declaration 
  of function 'atomic_cmpxchg' [-Werror=implicit-function-declaration]:  = 
  89:3
+ /scratch/kisskb/src/include/linux/atomic.h: error: implicit declaration 
  of function 'atomic_read' [-Werror=implicit-function-declaration]:  = 136:2
 
 sparc-allmodconfig
Not reproduceable here with linus latest. (-rc1 + two patches).
Can you help me with a pointer to the original build log?

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

Re: [PATCH v5 00/27] irq_domain generalization and rework

[Sam Ravnborg]

  
  /*
   * Please do not include this file in generic code.  There is currently
   * no requirement for any architecture to implement anything held
   * within this file.
   *
   * Thanks. --rmk
   */
  
  A quick grep indicates that we've lost this battle ;) Is the comments
  still true?  Should we stop discouraging inclusion of linux/irq.h? 
  Does anyone even know that it's discouraged ;)
 
 It's still true for any platform which hasn't been converted to genirq,
 as such a platform would not have asm/hw_irq.h.

In-tree only s390 is not using genirq.
All the rest are converted and provide hw_irq.h -
most of the new archs provide hw_irq via asm-generic so it does
not show up unless you look in the Kbuild file,.

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

Re: [PATCH] powerpc: stop exporting irq_map

[Sam Ravnborg]

 +
  irq_hw_number_t virq_to_hw(unsigned int virq)
  {
   return irq_map[virq].hwirq;
  }
  EXPORT_SYMBOL_GPL(virq_to_hw);

General comment...
kernel/irq/* denote the virtual irq numbers irq.
powerpc uses virq for the same.

Sometimes it is confusing that two different names is used
for the same thing.

The intent of this patch is to centralize access to a common data structure,
but this could also be a good candidate for a terminology shift if
this is considered.

git grep virq shows a lot of hits - especially in arm, powerpc.
So what I suggest is obviously only a small step in the direction
of using irq all over.

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

Re: [PATCH 0/3] dt: documentation reorganization

[Sam Ravnborg]

On Mon, Jan 31, 2011 at 12:44:41AM -0700, Grant Likely wrote:
 This series reorganizes and cleans up the device tree documentation
 to make the directory useful for non-powerpc users.

Looks better than first try. But the structure is very flat.
Did you consider an arch/* for all arch specific stuff?

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

Re: [PATCH 0/3] dt: documentation reorganization

[Sam Ravnborg]

On Mon, Jan 31, 2011 at 02:01:31PM -0700, Grant Likely wrote:
 On Mon, Jan 31, 2011 at 09:34:49PM +0100, Sam Ravnborg wrote:
  On Mon, Jan 31, 2011 at 12:44:41AM -0700, Grant Likely wrote:
   This series reorganizes and cleans up the device tree documentation
   to make the directory useful for non-powerpc users.
  
  Looks better than first try. But the structure is very flat.
  Did you consider an arch/* for all arch specific stuff?
 
 Yes I did; but this stuff is pretty sparse so I didn't want to create
 a whole lot of levels when it isn't warranted.  If really required in
 the future I'll split the arch and drivers into two separate
 directories.  In the mean time I'm going to stick with this.
OK.

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

Re: [PATCH] dt: Move device tree documentation out of powerpc directory

[Sam Ravnborg]

On Wed, Jan 26, 2011 at 10:20:34AM -0700, Grant Likely wrote:
 The device tree is used by more than just PowerPC.  Make the documentation
 directory available to all.

How does the planned stucture look like for this
new directory?
I see that in your move the architecture is dropped.
But some of these looks like they are arch candidates.
The 4xx/* for example looks like powerpc stuff that
should be in a powerpc directory.

Another is sja1000. This is obviously a can driver.
So how about mirroring the kernel structure:

Proposal:

drivers/net/can   === sja1000.txt
drivers/ata/  === fsl/sata.txt
arch/powerpc  === powerpc specific stuff

This is a bit more effort but then we end
up with a well-known structure.

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

Re: [PATCH 1/4] of: Add support for linking device tree blobs into vmlinux

[Sam Ravnborg]

On Mon, Dec 06, 2010 at 09:35:59AM -0800, dirk.brande...@gmail.com wrote:
 From: Dirk Brandewie dirk.brande...@gmail.com
 
 This patch adds support for linking device tree blob(s) into
 vmlinux. Modifies asm-generic/vmlinux.lds.h to add linking
 .dtb sections into vmlinux. To maintain compatiblity with the of/fdt
 driver code platforms MUST copy the blob to a non-init memory location
 before the kernel frees the .init.* sections in the image.
 
 Modifies scripts/Makefile.lib to add a kbuild command to
 compile DTS files to device tree blobs and a rule to create objects to
 wrap the blobs for linking.
 
 STRUCT_ALIGNMENT is defined in vmlinux.lds.h for use in the rule to
 create wrapper objects for the dtb in Makefile.lib.  The
 STRUCT_ALIGN() macro in vmlinux.lds.h is modified to use the
 STRUCT_ALIGNMENT definition.
 
 The DTB's are placed on 32 byte boundries to allow parsing the blob
 with driver/of/fdt.c during early boot without having to copy the blob
 to get the structure alignment GCC expects.
 
 A DTB is linked in by adding the DTB object to the list of objects to
 be linked into vmlinux in the archtecture specific Makefile using
obj-y += foo.dtb.o
 
 Signed-off-by: Dirk Brandewie dirk.brande...@gmail.com
 ---
  Documentation/kbuild/makefiles.txt |   15 +++
  include/asm-generic/vmlinux.lds.h  |   15 ---
  scripts/Makefile.lib   |   21 -
  3 files changed, 47 insertions(+), 4 deletions(-)
 
 diff --git a/Documentation/kbuild/makefiles.txt 
 b/Documentation/kbuild/makefiles.txt
 index 0ef00bd..fc18bb1 100644
 --- a/Documentation/kbuild/makefiles.txt
 +++ b/Documentation/kbuild/makefiles.txt
 @@ -1136,6 +1136,21 @@ When kbuild executes, the following steps are followed 
 (roughly):
 resulting in the target file being recompiled for no
 obvious reason.
  
 +dtc
 + Create flattend device tree blob object suitable for linking
 + into vmlinux. Device tree blobs linked into vmlinux are placed
 + in an init section in the image. Platform code *must* copy the
 + blob to non-init memory prior to calling unflatten_device_tree().
 +
 + Example:
 + #arch/x86/platform/ce4100/Makefile
 + clean-files := *dtb.S
 +
 + DTC_FLAGS := -p 1024
 + obj-y += foo.dtb.o
 +
 + $(obj)/%.dtb: $(src)/%.dts
 + $(call if_changed,dtc)

When using if_changed you need to add your target to targets-y.
And you need to specify FORCE as a prerequisite to force kbuild
to use the if_changed logic.

The purpose of if_changed is to check if the commandlien has changed
and execute the command again also if the commandline has changed.
The simpler variant is $(call cmd,dtc) where you do not check
the command line and do not need FORCE.

 diff --git a/include/asm-generic/vmlinux.lds.h 
 b/include/asm-generic/vmlinux.lds.h
 index bd69d79..024d3b9 100644
 --- a/include/asm-generic/vmlinux.lds.h
 +++ b/include/asm-generic/vmlinux.lds.h
 @@ -23,7 +23,7 @@
   *   _etext = .;
   *
   *  _sdata = .;
 - *   RO_DATA_SECTION(PAGE_SIZE)
 +*RO_DATA_SECTION(PAGE_SIZE)
   *   RW_DATA_SECTION(...)
   *   _edata = .;
   *

Change seems wrong.

 @@ -67,7 +67,8 @@
   * Align to a 32 byte boundary equal to the
   * alignment gcc 4.5 uses for a struct
   */
 -#define STRUCT_ALIGN() . = ALIGN(32)
 +#define STRUCT_ALIGNMENT 32
 +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
  
  /* The actual configuration determine if the init/exit sections
   * are handled as text/data or they can be discarded (which
 @@ -146,6 +147,13 @@
  #define TRACE_SYSCALLS()
  #endif
  
 +
 +#define KERNEL_DTB() \
 + STRUCT_ALIGN(); \
 + VMLINUX_SYMBOL(__dtb_start) = .;\
 + *(.dtb.init.rodata) \
 + VMLINUX_SYMBOL(__dtb_end) = .;
 +
  /* .data section */
  #define DATA_DATA\
   *(.data)\
 @@ -468,7 +476,8 @@
   MCOUNT_REC()\
   DEV_DISCARD(init.rodata)\
   CPU_DISCARD(init.rodata)\
 - MEM_DISCARD(init.rodata)
 + MEM_DISCARD(init.rodata)\
 + KERNEL_DTB()
  
  #define INIT_TEXT\
   *(.init.text)   \
 diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
 index 4c72c11..937eabbb 100644
 --- a/scripts/Makefile.lib
 +++ b/scripts/Makefile.lib
 @@ -200,7 +200,26 @@ quiet_cmd_gzip = GZIP$@
  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9  $@) || \
   (rm -f $@ ; false)
  
 -
 +# DTC
 +#  
 

Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

[Sam Ravnborg]

On Tue, Nov 16, 2010 at 02:41:36PM -0800, dirk.brande...@gmail.com wrote:
 From: Dirk Brandewie dirk.brande...@gmail.com
 
 This patch adds support for linking device tree blobs into
 vmlinux. Modifies asm-generic/vmlinux.lds.h to add linking
 .dtb.init.rodata sections into the .init.data section of the vmlinux
 image. Modifies scripts/Makefile.lib to add a kbuild command to
 compile DTS files to device tree blobs and a rule to create objects to
 wrap the blobs for linking.
 
 The DTB's are placed on 32 byte boundries to allow parsing the blob
 with driver/of/fdt.c during early boot without having to copy the blob
 to get the structure alignment GCC expects.
 
 A DTB is linked in by adding the DTB object to the list of objects to
 be linked into vmlinux in the archtecture specific Makefile using
obj-y += foo.dtb.o
 
 Signed-off-by: Dirk Brandewie dirk.brande...@gmail.com
 ---
  include/asm-generic/vmlinux.lds.h |   19 +--
  scripts/Makefile.lib  |   20 
  2 files changed, 37 insertions(+), 2 deletions(-)

When you touch Makefiles in scripts/* it is always a good idea to cc:
kbuild maintainer on the patch - I have added Michal.

Support functionality in Makefile.lib is documented in
Documentation/kbuild/* - please add documentation there.

 
 diff --git a/include/asm-generic/vmlinux.lds.h 
 b/include/asm-generic/vmlinux.lds.h
 index bd69d79..ea671e7 100644
 --- a/include/asm-generic/vmlinux.lds.h
 +++ b/include/asm-generic/vmlinux.lds.h
 @@ -67,7 +67,14 @@
   * Align to a 32 byte boundary equal to the
   * alignment gcc 4.5 uses for a struct
   */
 -#define STRUCT_ALIGN() . = ALIGN(32)
 +#define STRUCT_ALIGNMENT 32
 +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
 +
 +/* Device tree blobs linked into the kernel need to have proper
 + * structure alignment to be parsed by the flat device tree library
 + * used in early boot
 +*/
 +#define DTB_ALIGNMENT STRUCT_ALIGNMENT

It has been discussed in another thread some time ago to move
to a general 32 byte alignment for everything in vmlinux.lds.h
So there is not much need for the specific DTB alignment.

 diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
 index 4c72c11..29db062 100644
 --- a/scripts/Makefile.lib
 +++ b/scripts/Makefile.lib
 @@ -200,6 +200,26 @@ quiet_cmd_gzip = GZIP$@
  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9  $@) || \
   (rm -f $@ ; false)
  
 +# DTC
 +#  
 ---
 +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 + @echo '#include asm-generic/vmlinux.lds.h'  $@
 + @echo '.section .dtb.init.rodata,a'  $@
 + @echo '.balign DTB_ALIGNMENT'  $@
 + @echo '.global __dtb_$(*F)_begin'  $@
 + @echo '__dtb_$(*F)_begin:'  $@
 + @echo '.incbin $ '  $@
 + @echo '__dtb_$(*F)_end:'  $@
 + @echo '.global __dtb_$(*F)_end'  $@
 + @echo '.balign DTB_ALIGNMENT'  $@


This will be noisy during build. Please use proper macors to supress output.


 +
 +DTC = $(objtree)/scripts/dtc/dtc
 +
 +quiet_cmd_dtc = DTC  $@
Please avoid tabs in the output - all other uses spaces. (There is a tab 
between DTC and $@)

 +  cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) 
 $(src)/dts/$*.dts

Looks strange. How about:
  cmd_dtc = $(DTC) -O dtb -o $@ -b 0 $(DTS_FLAGS) $

Then you avoid the hardcoded path in the rule too.


 +
 +$(obj)/%.dtb: $(src)/dts/%.dts
 + $(call if_changed,dtc)

This snippet belong in the file that uses this.
This is how we do for other rules like bzip etc.

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

Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

[Sam Ravnborg]

   +
   +DTC = $(objtree)/scripts/dtc/dtc
   +
   +quiet_cmd_dtc = DTC  $@
  Please avoid tabs in the output - all other uses spaces. (There is a tab 
  between DTC and $@)
  
   +  cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) 
   $(src)/dts/$*.dts
  
  Looks strange. How about:
cmd_dtc = $(DTC) -O dtb -o $@ -b 0 $(DTS_FLAGS) $
  
  Then you avoid the hardcoded path in the rule too.
  
  
   +
   +$(obj)/%.dtb: $(src)/dts/%.dts
   + $(call if_changed,dtc)
 
 The rule should be generic (not depend on the presence of a dts
 subdirectory.  Basically, the .dtb really should be generated in the
 same directory as the .dts file.  There is no reason for this rule to
 have special behaviour.
 
  
  This snippet belong in the file that uses this.
  This is how we do for other rules like bzip etc.
 
 This rule is intended to be generic and usable anywhere in the tree.
I understand this.
But only few people will recognize this so they see something like this:

obj-y := foo.dtb.o

And they look for a file named foo.dtb.S or foo.dtb.c.

If we spell it out then we have a better chance to allow
people to understand the relation ships between the files.


If we really want to hide this in scipts/Makefile.* then Makefile.build
is the logical places to do so.
Makefile.lib is supposed to be stuff that is relavent for more than
one Makefile in scripts/* but it has unfortunately also grown
some of the boot support stuff.

Today there is a single rule related to _shipped files.
We should not fill it up with additional rules - put them
Makefile.build if we really want to avoid them in boot/Makefile.

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

Re: [PATCH 2/2] rwsem: Move powerpc atomic-long based implementation to asm-generic

[Sam Ravnborg]

 diff --git a/include/asm-generic/rwsem-cmpxchg.h 
 b/include/asm-generic/rwsem-cmpxchg.h
 new file mode 100644
 index 000..2b1c859
 --- /dev/null
 +++ b/include/asm-generic/rwsem-cmpxchg.h
 @@ -0,0 +1,183 @@
 +#ifndef _RWSEM_CMPXCHG_H
 +#define _RWSEM_CMPXCHG_H
 +
 +#ifndef _LINUX_RWSEM_H
 +#error Please don't include asm/rwsem.h directly, use linux/rwsem.h 
 instead.
 +#endif
 +
 +#ifdef __KERNEL__

#ifdef __KERNEL__ is only relevant for exported headers.
For kernel only headers like this is does not make sense,
but it does not harm.

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

Re: 64-bit ppc rwsem

[Sam Ravnborg]

 diff --git a/arch/sparc/lib/Makefile b/arch/sparc/lib/Makefile
 index c4b5e03..fa4c3ea 100644
 --- a/arch/sparc/lib/Makefile
 +++ b/arch/sparc/lib/Makefile
 @@ -15,7 +15,7 @@ lib-$(CONFIG_SPARC32) += divdi3.o udivdi3.o
  lib-$(CONFIG_SPARC32) += copy_user.o locks.o
  lib-y += atomic_$(BITS).o
  lib-$(CONFIG_SPARC32) += lshrdi3.o ashldi3.o
 -lib-y += rwsem_$(BITS).o
 +lib-$(CONFIG_SPARC32) += rwsem_$(BITS).o

You could write this explicit as:

 +lib-$(CONFIG_SPARC32) += rwsem_32.o

As rwsem_64 is gone now.

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

Re: [PATCH] powerpc: fix typo in uImage target

[Sam Ravnborg]

On Mon, Aug 16, 2010 at 10:26:56AM +0200, Anatolij Gustschin wrote:
 Commit e32e78c5ee8aadef020fbaecbe6fb741ed9029fd
 (powerpc: fix build with make 3.82) introduced a
 typo in uImage target and broke building uImage:
 
 make: *** No rule to make target `uImage'.  Stop.
 
 Signed-off-by: Anatolij Gustschin ag...@denx.de
 Cc: stable sta...@kernel.org

blush...

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

[PATCH] powerpc: fix build with make 3.82

[Sam Ravnborg]

Thomas Backlund reported that the powerpc build broke with make 3.82.
It failed with the following message:

arch/powerpc/Makefile:183: *** mixed implicit and normal rules.  Stop.

The fix is to avoid mixing non-wildcard and wildcard targets.

Reported-by: Thomas Backlund t...@mandriva.org
Tested-by: Thomas Backlund t...@mandriva.org
Cc: Michal Marek mma...@suse.cz
Cc: stable sta...@kernel.org
Signed-off-by: Sam Ravnborg s...@ravnborg.org
---

Hi Ben / Paul.

This fixes powerc build with latest make version.
The patch is on top of 2.6.35.
But it is more of a coincidence that we see a make release
right now and this issue is also present in older kernels.
So I have added a Cc: stable sta...@kernel.org because
I consider this relevant for the stable kernel releases too.

@Michal - you got a copy as information only.
I fear we may see this bug for other parts of the kernel too.

Sam


diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 77cfe7a..5d2f17d 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -163,9 +163,11 @@ drivers-$(CONFIG_OPROFILE) += arch/powerpc/oprofile/
 # Default to zImage, override when needed
 all: zImage
 
-BOOT_TARGETS = zImage zImage.initrd uImage zImage% dtbImage% treeImage.% 
cuImage.% simpleImage.%
+# With make 3.82 we cannot mix normal and wildcard targets
+BOOT_TARGETS1 := zImage zImage.initrd uImaged
+BOOT_TARGETS2 := zImage% dtbImage% treeImage.% cuImage.% simpleImage.%
 
-PHONY += $(BOOT_TARGETS)
+PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2)
 
 boot := arch/$(ARCH)/boot
 
@@ -180,10 +182,16 @@ relocs_check: arch/powerpc/relocs_check.pl vmlinux
 zImage: relocs_check
 endif
 
-$(BOOT_TARGETS): vmlinux
+$(BOOT_TARGETS1): vmlinux
+   $(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+$(BOOT_TARGETS2): vmlinux
+   $(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+
+
+bootwrapper_install:
$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
-bootwrapper_install %.dtb:
+%.dtb:
$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
 define archhelp

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

Re: make 3.82 fails on powerpc defconfig update [was: Linux 2.6.35]

[Sam Ravnborg]

On Mon, Aug 02, 2010 at 11:51:11AM +0300, Thomas Backlund wrote:
 Hi,
 (please cc me as I'm not subscribed)

 updating from make 3.81 to 3.82 gets me this:

 [tho...@tmb linux-2.6.35]$ cp arch/powerpc/configs/ppc64_defconfig .config
 [tho...@tmb linux-2.6.35]$ LC_ALL=C make oldconfig ARCH=powerpc
 /mnt/work/2.6.35/linux-2.6.35/arch/powerpc/Makefile:183: *** mixed  
 implicit and normal rules.  Stop.

 The lines are:

 182:
 183: $(BOOT_TARGETS): vmlinux
 184: $(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst  
 %,$(boot)/%,$@)
 185:

 BOOT_TARGETS are defined on line 166 as:
 BOOT_TARGETS = zImage zImage.initrd uImage zImage% dtbImage% treeImage.%  
 cuImage.% simpleImage.%


 Now it's not a regression in the kernel as the same happends with the  
 2.6.34 tree too.

 (btw, the host I'm syncing the defconfig with is a x86_64 machine)


 Now, I dont know if this is intended breakage by the make update, or  
 if the Makefile needs to be updated

 Any ideas how to fix ?

This is in the category intended breakage.
We had a similar issue in the top-level Makefile which Paul (IIRC)
helped me to fix long time ago.

To fix popwerpc I suggest something along these lines.
[Note: I did not test it - please do so.

Sam

[PATCH] powerpc: fix build with make 3.82

Thomas Backlund reported that the powerpc build broke with make 3.82.
It failed with the following message:

arch/powerpc/Makefile:183: *** mixed implicit and normal rules.  Stop.

The fix is to avoid mixing non-wildcard and wildcard targets.

Reported-by: Thomas Backlund t...@mandriva.org
Cc: Michal Marek mma...@suse.cz
Signed-off-by: Sam Ravnborg s...@ravnborg.org
---
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 77cfe7a..ad88b21 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -163,9 +163,11 @@ drivers-$(CONFIG_OPROFILE) += arch/powerpc/oprofile/
 # Default to zImage, override when needed
 all: zImage
 
-BOOT_TARGETS = zImage zImage.initrd uImage zImage% dtbImage% treeImage.% 
cuImage.% simpleImage.%
+# With make 3.82 we cannot mix normal and wildcard targets
+BOOT_TARGETS1 := zImage zImage.initrd uImaged
+BOOT_TARGETS2 := zImage% dtbImage% treeImage.% cuImage.% simpleImage.%
 
-PHONY += $(BOOT_TARGETS)
+PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2)
 
 boot := arch/$(ARCH)/boot
 
@@ -180,7 +182,9 @@ relocs_check: arch/powerpc/relocs_check.pl vmlinux
 zImage: relocs_check
 endif
 
-$(BOOT_TARGETS): vmlinux
+$(BOOT_TARGETS1): vmlinux
+   $(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+$(BOOT_TARGETS2): vmlinux
$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
 bootwrapper_install %.dtb:
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: make 3.82 fails on powerpc defconfig update [was: Linux 2.6.35]

[Sam Ravnborg]

 
 Thanks, this seems to fix the first issue, but then I get the same erro on 
 the following line 190:
 
 190: bootwrapper_install %.dtb:
 191:$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 

Obviously - dunno how I missed that.
Updated patch below.

I will do a proper submission after you
confirm that powerpc build is working with make 3.82.

Sam

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 77cfe7a..ace7a3e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -163,9 +163,11 @@ drivers-$(CONFIG_OPROFILE) += arch/powerpc/oprofile/
 # Default to zImage, override when needed
 all: zImage
 
-BOOT_TARGETS = zImage zImage.initrd uImage zImage% dtbImage% treeImage.% 
cuImage.% simpleImage.%
+# With make 3.82 we cannot mix normal and wildcard targets
+BOOT_TARGETS1 := zImage zImage.initrd uImaged
+BOOT_TARGETS2 := zImage% dtbImage% treeImage.% cuImage.% simpleImage.%
 
-PHONY += $(BOOT_TARGETS)
+PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2)
 
 boot := arch/$(ARCH)/boot
 
@@ -180,10 +182,16 @@ relocs_check: arch/powerpc/relocs_check.pl vmlinux
 zImage: relocs_check
 endif
 
-$(BOOT_TARGETS): vmlinux
+$(BOOT_TARGETS1): vmlinux
+   $(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+$(BOOT_TARGETS2): vmlinux
+   $(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+
+
+bootwrapper_install
$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
-bootwrapper_install %.dtb:
+%.dtb:
$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
 define archhelp
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC][PATCH] kconfig: implement select with values

[Sam Ravnborg]

 
 Cute.  I didn't know this was possible.  I'll give it a try and see
 how it works for me.  Do override config options also pickup
 select/depends constraints applied by later definitions?  I think that
 would be necessary to kick out warnings when a defconfig selection
 isn't actually achievable.

kconfig allows one to add more properties to a config
symbol by defining the symbol again.

So:
config FOO
bool
prompt foo prompt
default y
help
  Help text

Is the same as:
config FOO
bool

config FOO
prompt foo prompt

config FOO
default y

config FOO
help
  help text

The abvoe four lines can be located in different files.

And likewise kconfig allows the same property to be
specified twice or more.

So it is OK to say:

config BAR
tristate bar prompt
default m
default y

Here kconfig just picks the first default is see.

And the example in my original mail uses the feature
that we can specify several defaults - and kconfig uses the first.


For choices the same is possible but then you need to use
a named choice - something that no one does today in the kernel.

choice CHOICE_X86_CPU
default M386

endchoice

Here we change the default to M386 which becomes the
selected if we use alldefconfig.

[Note: It does not work until we use named choices in arch/x86/Kconfig.cpu]

 
  Now in the original suggestion of Linus he used:
 
     KBUILD_KCONFIG=Mykconfig make allnoconfig
 
  And allnoconfig would make the modified defaults useless.
 
  But the soon-to-be-introduced alldefconfig is more sensible.
 
  alldefconfig uses default values for everything.
 
 I basically used defconfig in the prototype that I posted.  I think it
 works well for me, and I've got it integrated into the build targets
 (just like Stephen's earlier patch) without having to do a
 KBUILD_KCONFIG trick.

defconfig with an empty file gives same config as alldefconfig so
it makes sense. But we wanted to drop the use of defconfig files
in favour of files with Kconfig syntax.
So alldefconfig is just a better fit here.

[The above btw. also makes savedefconfig less usefull].

Sam

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

Re: [PATCH v3] of: Create asm-generic/of.h and provide default of_node_to_nid()

[Sam Ravnborg]

On Tue, Jul 27, 2010 at 03:34:01PM +0200, Arnd Bergmann wrote:
 On Tuesday 27 July 2010, Grant Likely wrote:
   I suggest to go back to v2 of your patch where you use asm-generic/of.h.
  
  Stephen suggested dropping asm-generic/of.h.  I'm happy to do it either way.
 
 I don't mind adding stuff to asm-generic, but I think in this case it would
 be easier to keep this in linux/of.h because there is nothing wrong with
 all architectures including it.
 
 Most files in asm-generic are there only for historical reasons, where some
 architectures use them but others don't. IMHO we should use the include/linux
 headers preferred for new stuff though.

Hi Arnd.

Thanks for this explanation. Obviously my previous post
about asm-generic was wrong (as Grant already indicated).

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

Re: [PATCH v3] of: Create asm-generic/of.h and provide default of_node_to_nid()

[Sam Ravnborg]

On Mon, Jul 26, 2010 at 04:04:55PM -0600, Grant Likely wrote:
 of_node_to_nid() is only relevant in a few architectures.  Don't force
 everyone to implement it anyway.  This patch also adds asm-generic/of.h
 which will be used to contain other overrideable symbols.
 
 Signed-off-by: Grant Likely grant.lik...@secretlab.ca
 ---
 
 Changes in v3: don't use asm-generic, just keep macros in of.h
 Changes in v2: address comments from sfr, add asm-generic/of.h

The use of asm-generic makes perfect sense.
This is how we usually deal with arch specific stuff.

With v3 of your patch we have a different result depending
on if we do:
#include linux/of.h

or we do:
#include asm/prom.h

This is undesireable.

I suggest to go back to v2 of your patch where you use asm-generic/of.h.

linux/of.h shall include asm/of.h
Then all archs shall have a of.h that may
include the asm-generic variant.


One patch to introduce of.h all over.
And a second patch to do the of_node_to_nid stuff would be appropriate.


 diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
 index da7dd63..dca25a5 100644
 --- a/arch/powerpc/include/asm/prom.h
 +++ b/arch/powerpc/include/asm/prom.h
 @@ -103,6 +103,11 @@ struct device_node *of_find_next_cache_node(struct 
 device_node *np);
  /* Get the MAC address */
  extern const void *of_get_mac_address(struct device_node *np);
  

This shall go in asm/of.h
 +#ifdef CONFIG_NUMA
 +extern int of_node_to_nid(struct device_node *device);
 +#define of_node_to_nid of_node_to_nid

This define is used to tell asm-generic/of.h that the arch has
a local definition - OK.

 +#endif
 +
  /**
   * of_irq_map_pci - Resolve the interrupt for a PCI device
   * @pdev:the device whose interrupt is to be resolved

 diff --git a/include/linux/of.h b/include/linux/of.h
 index b0756f3..cc936ca 100644
 --- a/include/linux/of.h
 +++ b/include/linux/of.h
 @@ -146,6 +146,11 @@ static inline unsigned long of_read_ulong(const __be32 
 *cell, int size)
  
  #define OF_BAD_ADDR  ((u64)-1)
  
 +#ifndef of_node_to_nid
 +static inline int of_node_to_nid(struct device_node *np) { return 0; }
 +#define of_node_to_nid of_node_to_nid

But I fail to see the purpose of this define.

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

Re: [RFC][PATCH] kconfig: implement select with values

[Sam Ravnborg]

On Tue, Jul 20, 2010 at 04:37:06AM +1000, Stephen Rothwell wrote:
 This is a fairly brute force approach to allowing a Kconfig select
 to specify an excplicit value for the selected config sybmol.
 
 The syntax of select is changed to:
 
   select symbol [expr] [if expr]
 
 The approach taken is to add a list of value, dependency expression
 pairs to the symbol and check them whenever the reverse depends (from
 a current-style select) are checked.

I do not see the need for this feature.
I have read most of the postings in the defconfig battle and
I saw Linus' select suggestion.

But we can do almost all of this today (or tomorrow).

Consider following Kconfig file:

config USB_SUPPORT
def_bool n

config NET
def_bool y

source arch/x86/Kconfig

We use the feature that we can specify the same
config option several times.
And the first default value that is visible
will be used.

So it allows us to enable and disable any option.

And if we for example want to set a specific LOG_BUF_SHIFT
we use the same trick:

config LOG_BUF_SHIFT
int
default 14


Now in the original suggestion of Linus he used:

KBUILD_KCONFIG=Mykconfig make allnoconfig

And allnoconfig would make the modified defaults useless.

But the soon-to-be-introduced alldefconfig is more sensible.

alldefconfig uses default values for everything.

So with alldefconfig I think we have all the features we need
to create sensible default configs in the Kconfig language.

And therefore I do not see the need for the extension to select.
For the same reason I did not look closer at the implementation.

Sam

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

Re: [PATCH] powerpc: fix .data..init_task output section

[Sam Ravnborg]

On Thu, Jul 22, 2010 at 07:50:08PM -0400, Sean MacLennan wrote:
 On Tue, 13 Jul 2010 11:50:24 +0200
 Sam Ravnborg s...@ravnborg.org wrote:
 
  From 851e645a7eee68380caaf026eb6d3be118876370 Mon Sep 17 00:00:00 2001
  From: Sam Ravnborg s...@ravnborg.org
  Date: Tue, 13 Jul 2010 11:39:42 +0200
  Subject: [PATCH] vmlinux.lds: fix .data..init_task output section
  (fix popwerpc boot)
  
  The .data..init_task output section was missing
  a load offset causing a popwerpc target to fail to boot.
  
  Sean MacLennan tracked it down to the definition of
  INIT_TASK_DATA_SECTION().
  
  There are only two users of INIT_TASK_DATA_SECTION()
  in the kernel today: cris and popwerpc.
  cris do not support relocatable kernels and is thus not
  impacted by this change.
  
  Fix INIT_TASK_DATA_SECTION() to specify load offset like
  all other output sections.
  
  Reported-by: Sean MacLennan smaclen...@pikatech.com
  Signed-off-by: Sam Ravnborg s...@ravnborg.org
  ---
  
  On the assumption that Sean reports that it fixes
  the warnings/boot issue here is a real patch.
  
  Ben - will you take it via the popwerpc tree
  or shall I ask Michal to take it via kbuild?
  
  Sam

Sorry for the bad initial subject line!
As Sean reported that the patch fixes his isuse it
deserves a:

Tested-by: Sean MacLennan smaclen...@pikatech.com

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

Re: section .data..init_task

[Sam Ravnborg]

On Mon, Jul 12, 2010 at 08:34:35PM -0400, Sean MacLennan wrote:
 On Mon, 28 Jun 2010 00:59:00 -0400
 Sean MacLennan smaclen...@pikatech.com wrote:
 
  Anybody else seeing these messages?
  
  ppc_4xxFP-ld: .tmp_vmlinux1: section .data..init_task lma 0xc0374000
  overlaps previous sections ppc_4xxFP-ld: .tmp_vmlinux2:
  section .data..init_task lma 0xc03a2000 overlaps previous sections
  ppc_4xxFP-ld: vmlinux: section .data..init_task lma 0xc03a2000
  overlaps previous sections
  
  Or does anybody know what they mean? They started showing up in
  2.6.35.
  
  Very easy to reproduce, so don't hesitate to ask for more info.
 
 I had a bit of time, so I tracked this down. This patch seems to be
 the culprit: http://lkml.org/lkml/2010/2/19/366
 
 Specifically, this code:
 
   /* The initial task and kernel stack */
 - .data.init_task : AT(ADDR(.data.init_task) - LOAD_OFFSET) {
 - INIT_TASK_DATA(THREAD_SIZE)
 - }
 + INIT_TASK_DATA_SECTION(THREAD_SIZE)
 
 If I change it back to:
 
   /* The initial task and kernel stack */
   .data..init_task : AT(ADDR(.data..init_task) - LOAD_OFFSET) {
   INIT_TASK_DATA(THREAD_SIZE)
   }
 
 not only do the warnings go away, but the kernel now boots again!

It looks like a missing AT() in the output section.
The following patch should also fix it.

Please test and let us know.

Thanks,
Sam


diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 48c5299..3c4bf03 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -435,7 +435,7 @@
  */
 #define INIT_TASK_DATA_SECTION(align)  \
. = ALIGN(align);   \
-   .data..init_task : {\
+   .data..init_task : AT(ADDR(.data..init_task) - LOAD_OFFSET) {   \
INIT_TASK_DATA(align)   \
}
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[

[Sam Ravnborg]

From 851e645a7eee68380caaf026eb6d3be118876370 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg s...@ravnborg.org
Date: Tue, 13 Jul 2010 11:39:42 +0200
Subject: [PATCH] vmlinux.lds: fix .data..init_task output section (fix popwerpc 
boot)

The .data..init_task output section was missing
a load offset causing a popwerpc target to fail to boot.

Sean MacLennan tracked it down to the definition of
INIT_TASK_DATA_SECTION().

There are only two users of INIT_TASK_DATA_SECTION()
in the kernel today: cris and popwerpc.
cris do not support relocatable kernels and is thus not
impacted by this change.

Fix INIT_TASK_DATA_SECTION() to specify load offset like
all other output sections.

Reported-by: Sean MacLennan smaclen...@pikatech.com
Signed-off-by: Sam Ravnborg s...@ravnborg.org
---

On the assumption that Sean reports that it fixes
the warnings/boot issue here is a real patch.

Ben - will you take it via the popwerpc tree
or shall I ask Michal to take it via kbuild?

Sam

 include/asm-generic/vmlinux.lds.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 48c5299..cdfff74 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -435,7 +435,7 @@
  */
 #define INIT_TASK_DATA_SECTION(align)  \
. = ALIGN(align);   \
-   .data..init_task : {\
+   .data..init_task :  AT(ADDR(.data..init_task) - LOAD_OFFSET) {  \
INIT_TASK_DATA(align)   \
}
 
-- 
1.6.0.6

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

Re: section .data..init_task

[Sam Ravnborg]

On Tue, Jul 13, 2010 at 11:26:10AM -0400, Sean MacLennan wrote:
 On Tue, 13 Jul 2010 10:54:19 +0200
 Sam Ravnborg s...@ravnborg.org wrote:
 
  It looks like a missing AT() in the output section.
  The following patch should also fix it.
  
  Please test and let us know.
  
  Thanks,
  Sam
 
 Applied the patch and it solves the problem. Thanks.

Thanks for the quick feedback!

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

[PATCH] powerpc: fix userspace build of ptrace.h

[Sam Ravnborg]

From ff056c080d2b0b93bac07ad71125fee701919f5e Mon Sep 17 00:00:00 2001
From: Sam Ravnborg s...@ravnborg.org
Date: Sun, 9 May 2010 08:52:31 +0200
Subject: [PATCH] powerpc: fix userspace build of ptrace.h

Build of ptrace.h failed for assembly because it
pulls in stdint.h.
Use exportable types (__u32, __u64) to avoid the dependency
on stdint.h.

Signed-off-by: Sam Ravnborg s...@ravnborg.org
Cc: Andrey Volkov avol...@varma-el.com
Cc: Dave Kleikamp sha...@linux.vnet.ibm.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
---

A better fix is to remove the use of stdint like the following patch does.
Note - I have not even build tested this patch!

Sam

 arch/powerpc/include/asm/ptrace.h |   32 ++--
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/ptrace.h 
b/arch/powerpc/include/asm/ptrace.h
index 9e2d84c..77bbc68 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -24,11 +24,7 @@
  * 2 of the License, or (at your option) any later version.
  */
 
-#ifdef __KERNEL__
 #include linux/types.h
-#else
-#include stdint.h
-#endif
 
 #ifndef __ASSEMBLY__
 
@@ -300,13 +296,13 @@ do {  
  \
 #ifndef __ASSEMBLY__
 
 struct ppc_debug_info {
-   uint32_t version;   /* Only version 1 exists to date */
-   uint32_t num_instruction_bps;
-   uint32_t num_data_bps;
-   uint32_t num_condition_regs;
-   uint32_t data_bp_alignment;
-   uint32_t sizeof_condition;  /* size of the DVC register */
-   uint64_t features;
+   __u32 version;  /* Only version 1 exists to date */
+   __u32 num_instruction_bps;
+   __u32 num_data_bps;
+   __u32 num_condition_regs;
+   __u32 data_bp_alignment;
+   __u32 sizeof_condition; /* size of the DVC register */
+   __u64 features;
 };
 
 #endif /* __ASSEMBLY__ */
@@ -322,13 +318,13 @@ struct ppc_debug_info {
 #ifndef __ASSEMBLY__
 
 struct ppc_hw_breakpoint {
-   uint32_t version;   /* currently, version must be 1 */
-   uint32_t trigger_type;  /* only some combinations allowed */
-   uint32_t addr_mode; /* address match mode */
-   uint32_t condition_mode;/* break/watchpoint condition flags */
-   uint64_t addr;  /* break/watchpoint address */
-   uint64_t addr2; /* range end or mask */
-   uint64_t condition_value;   /* contents of the DVC register */
+   __u32 version;  /* currently, version must be 1 */
+   __u32 trigger_type; /* only some combinations allowed */
+   __u32 addr_mode;/* address match mode */
+   __u32 condition_mode;   /* break/watchpoint condition flags */
+   __u64 addr; /* break/watchpoint address */
+   __u64 addr2;/* range end or mask */
+   __u64 condition_value;  /* contents of the DVC register */
 };
 
 #endif /* __ASSEMBLY__ */
-- 
1.6.0.6
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [patch] powerpc: build modules outside the kernel tree fails, if it was built using O=

[Sam Ravnborg]

On Thu, Sep 24, 2009 at 03:28:11PM +0400, Yuri Frolov wrote:
 Hello,
 
 here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
 This patch should correctly export crtsavres.o in order to make O= option 
 working.
 Please, consider to apply.

Hi Yuri.

I like the way you do the extra link in Makefile.modpost.
But you need to redo some parts as per comments below.

 
 
 Fix linking modules against crtsavres.o

Please elaborate more on what this commit does.

 
 Previously we got
   CC  drivers/char/hw_random/rng-core.mod.o
   LD [M]  drivers/char/hw_random/rng-core.ko
 /there/src/buildroot.git.ppc/build_powerpc_nofpu/staging_dir/usr/bin/powerpc-linux-uclibc-ld:
  arch/powerpc/lib/crtsavres.o: No such file: No such file or directory

Always good to include error messages.

   * Makefile (LDFLAGS_MODULE_PREREQ): New variable to hold prerequisite
   files for modules.
   * arch/powerpc/Makefile: add crtsavres.o to LDFLAGS_MODULE_PREREQ.
   * scripts/Makefile.modpost (cmd_as_o_S): Copy from Makefile.build.
 (cmd_ld_ko_o): Also link LDFLAGS_MODULE_PREREQ.
 Provide rule to build objects from assembler.
But this GNUism can go - we do not use it in the kernel.
 
 
 Signed-off-by:  Bernhard Reutner-Fischer  rep.dot@gmail.com
 Signed-off by:  Yuri Frolov yfro...@ru.mvista.com
 
 Makefile |2 ++
 arch/powerpc/Makefile|2 +-
 scripts/Makefile.modpost |   12 ++--
 3 files changed, 13 insertions(+), 3 deletions(-)
 
 diff -urpN -X linux-2.6/Documentation/dontdiff 
 linux-2.6/arch/powerpc/Makefile 
 linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile
 --- linux-2.6/arch/powerpc/Makefile   2009-09-17 20:04:31.0 +0400
 +++ linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile 2009-09-23 
 22:08:03.0 +0400
 @@ -93,7 +93,7 @@ else
   KBUILD_CFLAGS += $(call cc-option,-mtune=power4)
  endif
  else
 -LDFLAGS_MODULE   += arch/powerpc/lib/crtsavres.o
 +LDFLAGS_MODULE_PREREQ += arch/powerpc/lib/crtsavres.o
  endif

The naming sucks.
How about:

KBUILD_MODULE_LINK_SOURCE

This would tell the reader that this is source to be linked on a module.

And this is an arch specific thing so no need to preset it in top-level
Makefile.
But it is mandatory to include a description in Documentation/kbuild/kbuild.txt


 --- linux-2.6/scripts/Makefile.modpost2009-09-17 20:04:42.0 
 +0400
 +++ linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost  2009-09-23 
 22:15:00.0 +0400
 @@ -122,14 +122,22 @@ quiet_cmd_cc_o_c = CC  $@
cmd_cc_o_c = $(CC) $(c_flags) $(CFLAGS_MODULE) \
  -c -o $@ $
  
 -$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
 +quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
 +cmd_as_o_S   = $(CC) $(a_flags) $(AFLAGS_MODULE) -c -o $@ $

Align this so cmd_as_o_S is under each other - as we do for cmd_cc_o_c


 +
 +$(LDFLAGS_MODULE_PREREQ): %.o: %.S FORCE
 + $(Q)mkdir -p $(dir $@)
 + $(call if_changed_dep,as_o_S)
Good catch with the mkdir - needed for O= builds.
I think we shall wrap this in
ifdef KBUILD_MODULE_LINK_SOURCE
...
endif

So we do not have an empty rule when it is not defined.

Please fix up these things and resubmit.

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

Re: [patch] powerpc: build modules outside the kernel tree fails, if it was built using O=

[Sam Ravnborg]

On Sat, Sep 26, 2009 at 07:57:32AM +1000, Benjamin Herrenschmidt wrote:
 On Fri, 2009-09-25 at 21:45 +0200, Sam Ravnborg wrote:
  On Thu, Sep 24, 2009 at 03:28:11PM +0400, Yuri Frolov wrote:
   Hello,
   
   here is a corresponding bug: 
   http://bugzilla.kernel.org/show_bug.cgi?id=11143
   This patch should correctly export crtsavres.o in order to make O= option 
   working.
   Please, consider to apply.
  
  Hi Yuri.
  
  I like the way you do the extra link in Makefile.modpost.
  But you need to redo some parts as per comments below.
 
   
   Fix linking modules against crtsavres.o
  
  Please elaborate more on what this commit does.
 
 It's a support file that needs to be linked against every module
 (aka libgcc like) and thus needs to be built before any module.

Yes.

My point was that the next version of the changelog should
include this information - as well as kbuild.txt.

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

Re: [patch] powerpc: build modules outside the kernel tree fails, if it was built using O=

[Sam Ravnborg]

On Fri, Sep 25, 2009 at 11:12:21AM +1000, Benjamin Herrenschmidt wrote:
 On Thu, 2009-09-24 at 15:28 +0400, Yuri Frolov wrote:
  Hello,
  
  here is a corresponding bug: 
  http://bugzilla.kernel.org/show_bug.cgi?id=11143
  This patch should correctly export crtsavres.o in order to make O= option 
  working.
  Please, consider to apply.
  
  
  Fix linking modules against crtsavres.o
 
 Hi !
 
 This is the same patch you already posted as 
 
 
 [PATCH] Fix linking modules against
 crtsavres.o
 
 
 Or it's an update ?
 
 I've asked Sam to review it already since it affects the main kernel
 makefiles, waiting for his answer.
Saw the duplicates. Will get back to it tonight (morning here now).

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

Re: [PATCH] powerpc: Cleanup linker script using new linker script macros.

[Sam Ravnborg]

On Tue, Sep 22, 2009 at 11:18:09AM -0400, Tim Abbott wrote:
 Signed-off-by: Tim Abbott tabb...@ksplice.com
 Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
 Cc: Paul Mackerras pau...@samba.org
 Cc: linuxppc-...@ozlabs.org
 Cc: Sam Ravnborg s...@ravnborg.org

Look good.

Acked-by: Sam Ravnborg s...@ravnborg.org

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

Re: [PATCH] powerpc: minor Makefile simplification through use of cc-ifversion

[Sam Ravnborg]

On Thu, Jul 23, 2009 at 08:57:18PM +0200, Frans Pop wrote:
 Signed-off-by: Frans Pop elen...@planet.nl
Acked-by: Sam Ravnborg s...@ravnborg.org
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2] fbdev: work around old compiler bug

[Sam Ravnborg]

On Tue, Jun 23, 2009 at 12:45:05PM -0400, Kyle McMartin wrote:
 On Tue, Jun 23, 2009 at 03:44:28PM +1000, Stephen Rothwell wrote:
  When building with a 4.1.x compiler on powerpc64 (at least) we get
  this error:
  
  drivers/video/logo/logo_linux_mono.c:81: error: logo_linux_mono causes a 
  section type conflict
  
  This was introduced by commit ae52bb2384f721562f15f719de1acb8e934733cb
  (fbdev: move logo externs to header file).  This is a partial revert
  of that commit sufficient to not hit the compiler bug.
  
 
 We're seeing similar issues with 4.3 on parisc (the case I saw today was
 in fs/nfs/nfsroot.c...) Any ideas on the actual culprit or is it just
 gcc being unfriendly?

Al analysed this some time ago.
When we say something is const then _sometimes_ gcc annotate the
section as const(*) - sometimes not.
So if we have two variables/functions annotated __*const and
gcc decides to annotate the section const only in one case
we get a section type conflict.

(*) it is named something else I do not recall at the moment
in linker language.

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

Re: [PATCH] fbdev: work around old compiler bug

[Sam Ravnborg]

On Mon, Jun 22, 2009 at 06:34:15PM +1000, Stephen Rothwell wrote:
 On Mon, 22 Jun 2009 18:04:20 +1000 Stephen Rothwell s...@canb.auug.org.au 
 wrote:
 
  When building with a 4.1.x compiler on powerpc64 (at least) we get
  this error:
 
 Ignore this, it causes more problems:
 
 drivers/video/logo/logo_linux_clut224.c:548: error: logo_linux_clut224_clut 
 causes a section type conflict
 
 I'll work on a better solution.

I have no time to experiemnt atm.
But I have seen this before.
It happens when we mix up const and non-const stuff.

I would assume the simple solution is to replace _initconst with _initdata
for all users.
If you get it to work with powerpc then it works for everyone (or so it is 
usually).

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

kbuild, modpost: fix non-allocatable section warnings

[Sam Ravnborg]

It turned out there were at least three sources of
the non-allocatable section warning.

1) SUSE specific .comment section
2) endianness issues in elf header
3) additional debug sections

I have updated kbuild-fixes.git with
patches for all of the above.

But as we have seen three independent sources of this
warning within short time I expect there may be
yet-to-be-discovered issues.

The patches accumulated so far all looks trivially correct
and Jean and Sean has been quick to test them.
So unless something unexpected shows up I plan to push them
within a day or two.

I would like to see them in -next for a day or two first
in the hope that if other architectures has troubles
they will report this before it hits upstream.

Patches will follow for reference / testing.

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

[PATCH 1/3] kbuild, modpost: fix unexpected non-allocatable section when cross compiling

[Sam Ravnborg]

From 7d875a02864a35532543897195dfea2235815df8 Mon Sep 17 00:00:00 2001
From: Anders Kaseorg ande...@mit.edu
Date: Sun, 3 May 2009 22:02:55 +0200
Subject: [PATCH 1/3] kbuild, modpost: fix unexpected non-allocatable section 
when cross compiling

The missing TO_NATIVE(sechdrs[i].sh_flags) was causing many
unexpected non-allocatable section warnings when cross-compiling
for an architecture with a different endianness.

Fix endianness of all the fields in the ELF header and
section headers, not just some of them so we are not
hit by this anohter time.

Signed-off-by: Anders Kaseorg ande...@mit.edu
Reported-by: Sean MacLennan smaclen...@pikatech.com
Tested-by: Sean MacLennan smaclen...@pikatech.com
Signed-off-by: Sam Ravnborg s...@ravnborg.org
---
 scripts/mod/modpost.c |   35 +++
 1 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 936b6f8..a5c17db 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -384,11 +384,19 @@ static int parse_elf(struct elf_info *info, const char 
*filename)
return 0;
}
/* Fix endianness in ELF header */
-   hdr-e_shoff= TO_NATIVE(hdr-e_shoff);
-   hdr-e_shstrndx = TO_NATIVE(hdr-e_shstrndx);
-   hdr-e_shnum= TO_NATIVE(hdr-e_shnum);
-   hdr-e_machine  = TO_NATIVE(hdr-e_machine);
-   hdr-e_type = TO_NATIVE(hdr-e_type);
+   hdr-e_type  = TO_NATIVE(hdr-e_type);
+   hdr-e_machine   = TO_NATIVE(hdr-e_machine);
+   hdr-e_version   = TO_NATIVE(hdr-e_version);
+   hdr-e_entry = TO_NATIVE(hdr-e_entry);
+   hdr-e_phoff = TO_NATIVE(hdr-e_phoff);
+   hdr-e_shoff = TO_NATIVE(hdr-e_shoff);
+   hdr-e_flags = TO_NATIVE(hdr-e_flags);
+   hdr-e_ehsize= TO_NATIVE(hdr-e_ehsize);
+   hdr-e_phentsize = TO_NATIVE(hdr-e_phentsize);
+   hdr-e_phnum = TO_NATIVE(hdr-e_phnum);
+   hdr-e_shentsize = TO_NATIVE(hdr-e_shentsize);
+   hdr-e_shnum = TO_NATIVE(hdr-e_shnum);
+   hdr-e_shstrndx  = TO_NATIVE(hdr-e_shstrndx);
sechdrs = (void *)hdr + hdr-e_shoff;
info-sechdrs = sechdrs;
 
@@ -402,13 +410,16 @@ static int parse_elf(struct elf_info *info, const char 
*filename)
 
/* Fix endianness in section headers */
for (i = 0; i  hdr-e_shnum; i++) {
-   sechdrs[i].sh_type   = TO_NATIVE(sechdrs[i].sh_type);
-   sechdrs[i].sh_offset = TO_NATIVE(sechdrs[i].sh_offset);
-   sechdrs[i].sh_size   = TO_NATIVE(sechdrs[i].sh_size);
-   sechdrs[i].sh_link   = TO_NATIVE(sechdrs[i].sh_link);
-   sechdrs[i].sh_name   = TO_NATIVE(sechdrs[i].sh_name);
-   sechdrs[i].sh_info   = TO_NATIVE(sechdrs[i].sh_info);
-   sechdrs[i].sh_addr   = TO_NATIVE(sechdrs[i].sh_addr);
+   sechdrs[i].sh_name  = TO_NATIVE(sechdrs[i].sh_name);
+   sechdrs[i].sh_type  = TO_NATIVE(sechdrs[i].sh_type);
+   sechdrs[i].sh_flags = TO_NATIVE(sechdrs[i].sh_flags);
+   sechdrs[i].sh_addr  = TO_NATIVE(sechdrs[i].sh_addr);
+   sechdrs[i].sh_offset= TO_NATIVE(sechdrs[i].sh_offset);
+   sechdrs[i].sh_size  = TO_NATIVE(sechdrs[i].sh_size);
+   sechdrs[i].sh_link  = TO_NATIVE(sechdrs[i].sh_link);
+   sechdrs[i].sh_info  = TO_NATIVE(sechdrs[i].sh_info);
+   sechdrs[i].sh_addralign = TO_NATIVE(sechdrs[i].sh_addralign);
+   sechdrs[i].sh_entsize   = TO_NATIVE(sechdrs[i].sh_entsize);
}
/* Find symbol table. */
for (i = 1; i  hdr-e_shnum; i++) {
-- 
1.6.3.rc3.40.g75b44

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

[PATCH 2/3] kbuild, modpost: fix unexpected non-allocatable warning with SUSE gcc

[Sam Ravnborg]

From 028ecebdd83cc4a7f8c7e96e28a5537d2ac98dae Mon Sep 17 00:00:00 2001
From: Sam Ravnborg s...@ravnborg.org
Date: Sun, 3 May 2009 22:17:37 +0200
Subject: [PATCH 2/3] kbuild, modpost: fix unexpected non-allocatable warning 
with SUSE gcc

Jean reported that he saw one warning for each module like the one below:
WARNING: arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.o (.comment.SUSE.OPTs): 
unexpected non-allocatable section.

The warning appeared with the improved version of the
check of the flags in the sections.

That check already ignored sections named .comment - but SUSE store
additional info in the comment section and has named it in a SUSE
specific way. Therefore modpost failed to ignore the section.

The fix is to extend the pattern so we ignore all sections
that start with the name .comment..

Signed-off-by: Sam Ravnborg s...@ravnborg.org
Reported-by: Jean Delvare kh...@linux-fr.org
Tested-by: Jean Delvare kh...@linux-fr.org
---
 scripts/mod/modpost.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index a5c17db..268d457 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -727,7 +727,7 @@ int match(const char *sym, const char * const pat[])
 
 /* sections that we do not want to do full section mismatch check on */
 static const char *section_white_list[] =
-   { .comment, .debug*, .stab*, .note*, .got*, .toc*, NULL };
+   { .comment*, .debug*, .stab*, .note*, .got*, .toc*, NULL };
 
 /*
  * This is used to find sections missing the SHF_ALLOC flag.
-- 
1.6.3.rc3.40.g75b44

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

[PATCH 3/3] kbuild, modpost: fix unexpected non-allocatable warning with mips

[Sam Ravnborg]

From 4391ed6aa9a38cdfb48addd7a9b24a2ff099b1a7 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg s...@ravnborg.org
Date: Mon, 4 May 2009 13:05:26 +0200
Subject: [PATCH 3/3] kbuild, modpost: fix unexpected non-allocatable warning 
with mips

mips emit the following debug sections:
.mdebug* and .pdr

They were included in the check for non-allocatable section
and caused modpost to warn.

Manuel Lauss suggested to fix this by adding the relevant
sections to the list of sections we do not check.

Signed-off-by: Sam Ravnborg s...@ravnborg.org
Reported-by: Manuel Lauss m...@roarinelk.homelinux.net
---
 scripts/mod/modpost.c |   12 +++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 268d457..161b784 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -727,7 +727,17 @@ int match(const char *sym, const char * const pat[])
 
 /* sections that we do not want to do full section mismatch check on */
 static const char *section_white_list[] =
-   { .comment*, .debug*, .stab*, .note*, .got*, .toc*, NULL };
+{
+   .comment*,
+   .debug*,
+   .mdebug*,/* alpha, score, mips etc. */
+   .pdr,/* alpha, score, mips etc. */
+   .stab*,
+   .note*,
+   .got*,
+   .toc*,
+   NULL
+};
 
 /*
  * This is used to find sections missing the SHF_ALLOC flag.
-- 
1.6.3.rc3.40.g75b44

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

Re: unexpected non-allocatable section

[Sam Ravnborg]

On Sun, May 03, 2009 at 09:33:16PM +0200, Wolfgang Denk wrote:
 Dear Sean MacLennan,
 
 In message 20090503123959.0cc5c...@lappy.seanm.ca you wrote:
  
   What gcc, binutils versions and config are you using?
  
  gcc version 4.0.0 (DENX ELDK 4.1 4.0.0)
  GNU assembler version 2.16.1 (powerpc-linux) using BFD version 2.16.1
  
  And this is all running on the Warp with a 440EP.
 
 Which exact commands did you use to build the kenrel, and how did you
 set (and export?) the CROSS_COMPILE environment variable?
 
 The thing is, that I cannot reproduce this - I tested it with
 v2.6.30-rc4, both with ELDK 4.1 (as you) and ELDK 4.2.
 
 Both build the kernel image without any such warnings.

Anders already found the cause of this - it
was a missing endian conversion.
So you need to run this on a little endian target to
see it. And you need to do a full kernel build
so we run modpsot on vmlinux.

I will push the patch in a few minutes.

For reference it is below:

Sam

From: Anders Kaseorg ande...@mit.edu
Subject: [PATCH] kbuild, modpost: fix unexpected non-allocatable section when 
cross compiling

The missing TO_NATIVE(sechdrs[i].sh_flags) was causing many
unexpected non-allocatable section warnings when cross-compiling
for an architecture with a different endianness.

Fix endianness of all the fields in the ELF header and
section headers, not just some of them so we are not
hit by this anohter time.

Signed-off-by: Anders Kaseorg ande...@mit.edu
Signed-off-by: Sam Ravnborg s...@ravnborg.org
---
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 936b6f8..a5c17db 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -384,11 +384,19 @@ static int parse_elf(struct elf_info *info, const char 
*filename)
return 0;
}
/* Fix endianness in ELF header */
-   hdr-e_shoff= TO_NATIVE(hdr-e_shoff);
-   hdr-e_shstrndx = TO_NATIVE(hdr-e_shstrndx);
-   hdr-e_shnum= TO_NATIVE(hdr-e_shnum);
-   hdr-e_machine  = TO_NATIVE(hdr-e_machine);
-   hdr-e_type = TO_NATIVE(hdr-e_type);
+   hdr-e_type  = TO_NATIVE(hdr-e_type);
+   hdr-e_machine   = TO_NATIVE(hdr-e_machine);
+   hdr-e_version   = TO_NATIVE(hdr-e_version);
+   hdr-e_entry = TO_NATIVE(hdr-e_entry);
+   hdr-e_phoff = TO_NATIVE(hdr-e_phoff);
+   hdr-e_shoff = TO_NATIVE(hdr-e_shoff);
+   hdr-e_flags = TO_NATIVE(hdr-e_flags);
+   hdr-e_ehsize= TO_NATIVE(hdr-e_ehsize);
+   hdr-e_phentsize = TO_NATIVE(hdr-e_phentsize);
+   hdr-e_phnum = TO_NATIVE(hdr-e_phnum);
+   hdr-e_shentsize = TO_NATIVE(hdr-e_shentsize);
+   hdr-e_shnum = TO_NATIVE(hdr-e_shnum);
+   hdr-e_shstrndx  = TO_NATIVE(hdr-e_shstrndx);
sechdrs = (void *)hdr + hdr-e_shoff;
info-sechdrs = sechdrs;
 
@@ -402,13 +410,16 @@ static int parse_elf(struct elf_info *info, const char 
*filename)
 
/* Fix endianness in section headers */
for (i = 0; i  hdr-e_shnum; i++) {
-   sechdrs[i].sh_type   = TO_NATIVE(sechdrs[i].sh_type);
-   sechdrs[i].sh_offset = TO_NATIVE(sechdrs[i].sh_offset);
-   sechdrs[i].sh_size   = TO_NATIVE(sechdrs[i].sh_size);
-   sechdrs[i].sh_link   = TO_NATIVE(sechdrs[i].sh_link);
-   sechdrs[i].sh_name   = TO_NATIVE(sechdrs[i].sh_name);
-   sechdrs[i].sh_info   = TO_NATIVE(sechdrs[i].sh_info);
-   sechdrs[i].sh_addr   = TO_NATIVE(sechdrs[i].sh_addr);
+   sechdrs[i].sh_name  = TO_NATIVE(sechdrs[i].sh_name);
+   sechdrs[i].sh_type  = TO_NATIVE(sechdrs[i].sh_type);
+   sechdrs[i].sh_flags = TO_NATIVE(sechdrs[i].sh_flags);
+   sechdrs[i].sh_addr  = TO_NATIVE(sechdrs[i].sh_addr);
+   sechdrs[i].sh_offset= TO_NATIVE(sechdrs[i].sh_offset);
+   sechdrs[i].sh_size  = TO_NATIVE(sechdrs[i].sh_size);
+   sechdrs[i].sh_link  = TO_NATIVE(sechdrs[i].sh_link);
+   sechdrs[i].sh_info  = TO_NATIVE(sechdrs[i].sh_info);
+   sechdrs[i].sh_addralign = TO_NATIVE(sechdrs[i].sh_addralign);
+   sechdrs[i].sh_entsize   = TO_NATIVE(sechdrs[i].sh_entsize);
}
/* Find symbol table. */
for (i = 1; i  hdr-e_shnum; i++) {
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: unexpected non-allocatable section

[Sam Ravnborg]

 I ran the tests on a LE machine:
 
 Linux gemini.denx.de 2.6.27.15-170.2.24.fc10.i686 #1 SMP Wed Feb 11 23:58:12 
 EST 2009 i686 i686 i386 GNU/Linux
 
  I will push the patch in a few minutes.
  
  For reference it is below:
  
  Sam
  
  From: Anders Kaseorg ande...@mit.edu
  Subject: [PATCH] kbuild, modpost: fix unexpected non-allocatable section 
  when cross compiling
  
  The missing TO_NATIVE(sechdrs[i].sh_flags) was causing many
  unexpected non-allocatable section warnings when cross-compiling
  for an architecture with a different endianness.
 
 I'm confused. Why didn't I see this, then?

Maybe they just scrolled past the screen first time?
You need to do rm vmlinux.o to reproduce it.

The warnings are shown when we do section mismatch analysis on vmlinux.o
which is part of the final steps in creating vmlinux.
But you will force the check again if you only delete vmlinux.

You need to delete vmlinux.o to see them.

I hope this is the explanation - otherwise I have no good idea.

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

Re: unexpected non-allocatable section

[Sam Ravnborg]

On Sun, May 03, 2009 at 04:32:41PM -0400, Sean MacLennan wrote:
 On Sun, 3 May 2009 22:07:01 +0200
 Sam Ravnborg s...@ravnborg.org wrote:
 
  On Sun, May 03, 2009 at 09:33:16PM +0200, Wolfgang Denk wrote:
 
   Which exact commands did you use to build the kenrel, and how did
   you set (and export?) the CROSS_COMPILE environment variable?
 
 export CROSS_COMPILE=ppc_4xxFP-
 export ARCH=powerpc
 
 The toolchain is in my path
 
   The thing is, that I cannot reproduce this - I tested it with
   v2.6.30-rc4, both with ELDK 4.1 (as you) and ELDK 4.2.
   
   Both build the kernel image without any such warnings.
  
  Anders already found the cause of this - it
  was a missing endian conversion.
  So you need to run this on a little endian target to
  see it. And you need to do a full kernel build
  so we run modpsot on vmlinux.
  
  I will push the patch in a few minutes.
 
 That patch gets rid of the warnings.

Thanks for the quick testing. I will add a Tested-by: if I rebase the tree.

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

Re: [PATCH v2 0/6] macros for section name cleanup

[Sam Ravnborg]

On Thu, Apr 30, 2009 at 03:54:07PM -0400, Tim Abbott wrote:
 (this patch series differs from v1 only in the CC list; some of the
 architecture lists I sent the previous one to are moderated against
 non-members; all replies should go to this version).
 
 Here are the architecture-independent macro definitions needed for
 to clean up the kernel's section names.  The overall diffstat from
 this section name cleanup project is:
 
  96 files changed, 261 insertions(+), 503 deletions(-)
 
 The decrease results from removing a lot of redundancy in the linker
 scripts.
 
 The long-term goal here is to add support for building the kernel with
 -ffunction-sections -fdata-sections.  This requires renaming all the
 magic section names in the kernel of the form .text.foo, .data.foo,
 .bss.foo, and .rodata.foo to not have collisions with sections
 generated for code like:
 
 static int nosave = 0; /* -fdata-sections places in .data.nosave */
 static void head(); /* -ffunction-sections places in .text.head */
 
 Sam Ravnborg proposed that rather than just renaming all the sections
 outright, we should start by first getting more control over the
 section names used in the kernel so that we can later rename sections
 without touching too many files.  These patch series implement that
 cleanup.  Later, there will be another patch series to actually rename
 the sections.
 
 I'm hoping we can get just these macro definitions into 2.6.30 so that
 the arch maintainers don't have to grab the macro definitions for
 their trees while reviewing the patches for 2.6.31.
 
 Shortly, I'm going to send one patch series for each of the
 architectures updating those architectures to use these new macros
 (and otherwise cleaning up section names on those architectures).

Hi Tim.

We agreed to get the common stuff and one architecture done before
proceeding with the rest.
Please stick to that plan so we avoid patch-bombing lkml + maintainers.

When we have this ready it will be a simple one-patch-per-arch to cover
the rest.

I will comment on your common patches for now.

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

Re: [PATCH v2 1/6] Add new macros for page-aligned data and bss sections.

[Sam Ravnborg]

On Thu, Apr 30, 2009 at 03:54:08PM -0400, Tim Abbott wrote:
 This patch is preparation for replacing most uses of
 .bss.page_aligned and .data.page_aligned in the kernel with
 macros, so that the section name can later be changed without having
 to touch a lot of the kernel.
 
 The long-term goal here is to be able to change the kernel's magic
 section names to those that are compatible with -ffunction-sections
 -fdata-sections.  This requires renaming all magic sections with names
 of the form .data.foo.
 
 Signed-off-by: Tim Abbott tabb...@mit.edu
 Cc: Sam Ravnborg s...@ravnborg.org
 Acked-by: David Howells dhowe...@redhat.com
 ---
  include/asm-generic/vmlinux.lds.h |8 
  include/linux/linkage.h   |9 +
  2 files changed, 17 insertions(+), 0 deletions(-)
 
 diff --git a/include/asm-generic/vmlinux.lds.h 
 b/include/asm-generic/vmlinux.lds.h
 index 89853bc..3d88c87 100644
 --- a/include/asm-generic/vmlinux.lds.h
 +++ b/include/asm-generic/vmlinux.lds.h
 @@ -116,6 +116,14 @@
   FTRACE_EVENTS() \
   TRACE_SYSCALLS()
  
 +#define PAGE_ALIGNED_DATA\
 + . = ALIGN(PAGE_SIZE);   \
 + *(.data.page_aligned)
 +
 +#define PAGE_ALIGNED_BSS \
 + . = ALIGN(PAGE_SIZE);   \
 + *(.bss.page_aligned)
 +
  #define RO_DATA(align)   
 \
   . = ALIGN((align)); \
   .rodata   : AT(ADDR(.rodata) - LOAD_OFFSET) {   \
 diff --git a/include/linux/linkage.h b/include/linux/linkage.h
 index fee9e59..af051fc 100644
 --- a/include/linux/linkage.h
 +++ b/include/linux/linkage.h
 @@ -22,6 +22,15 @@
  #define __page_aligned_bss   __section(.bss.page_aligned) 
 __aligned(PAGE_SIZE)
  
  /*
 + * For assembly routines.
 + *
 + * Note when using these that you must specify the appropriate
 + * alignment directives yourself
 + */
 +#define __PAGE_ALIGNED_DATA  .section .data.page_aligned, aw, @progbits
 +#define __PAGE_ALIGNED_BSS   .section .bss.page_aligned, aw, @nobits

The above will work on most architectures but fails (silently?) on arm.
arm uses %{progbits,nobits} where all other uses @{nobits,progbits}.

I know we do not use page_aligned in arm assembler code for now,
but if we do then it should work.
It is my understanding that the linker will automatically
assume nobits for section names starting with .bss and likewise
progbits for section names starting with .data - so we can leave them out?


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

Re: [PATCH v2 2/6] Add new NOSAVE_DATA linker script macro.

[Sam Ravnborg]

On Thu, Apr 30, 2009 at 03:54:09PM -0400, Tim Abbott wrote:
 This patch is preparation for replacing most .data.nosave in the
 kernel with macros, so that the section name can later be changed
 without having to touch a lot of the kernel.
 
 The long-term goal here is to be able to change the kernel's magic
 section names to those that are compatible with -ffunction-sections
 -fdata-sections.  This requires renaming all magic sections with names
 of the form .data.foo.
 
 Signed-off-by: Tim Abbott tabb...@mit.edu
 Cc: Sam Ravnborg s...@ravnborg.org
 ---
  include/asm-generic/vmlinux.lds.h |7 +++
  1 files changed, 7 insertions(+), 0 deletions(-)
 
 diff --git a/include/asm-generic/vmlinux.lds.h 
 b/include/asm-generic/vmlinux.lds.h
 index 3d88c87..f5ebd2b 100644
 --- a/include/asm-generic/vmlinux.lds.h
 +++ b/include/asm-generic/vmlinux.lds.h
 @@ -124,6 +124,13 @@
   . = ALIGN(PAGE_SIZE);   \
   *(.bss.page_aligned)
  
 +#define NOSAVE_DATA  \
 + . = ALIGN(PAGE_SIZE);   \
 + __nosave_begin = .; \
 + *(.data.nosave) \
 + . = ALIGN(PAGE_SIZE);   \
 + __nosave_end = .;
 +

You need to use:
VMLINUX_SYMBOL(__nosave_begin) = .;

Otherwise architectures such as m68k wil break as they
add a leading underscore.

See other symbols that is defined inside vmlinux.lds.h

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

Re: [PATCH 6/6] Add support for __read_mostly to linux/cache.h

[Sam Ravnborg]

On Thu, Apr 30, 2009 at 03:32:36PM -0400, Tim Abbott wrote:
 Signed-off-by: Tim Abbott tabb...@mit.edu
 ---
  include/linux/cache.h |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/cache.h b/include/linux/cache.h
 index 97e2488..99d8a6f 100644
 --- a/include/linux/cache.h
 +++ b/include/linux/cache.h
 @@ -13,7 +13,13 @@
  #endif
  
  #ifndef __read_mostly
 +#ifdef CONFIG_HAVE_READ_MOSTLY_DATA
 +#define __read_mostly __attribute__((__section__(.data.read_mostly)))
 +#define __READ_MOSTLY .section .data.read_mostly, aw
 +#else
  #define __read_mostly
 +#define __READ_MOSTLY
 +#endif /* CONFIG_HAVE_READ_MOSTLY_DATA */
  #endif

Are there any specific reason why we do not support read_mostly on all
architectures?

read_mostly is about grouping rarely written data together
so what is needed is to introduce this section in the remaining
archtectures.

Christoph - git log says you did the inital implmentation.
Do you agree?

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