* [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
@ 2015-01-01 17:02 Rickard Strandqvist
2015-01-04 7:21 ` Finn Thain
0 siblings, 1 reply; 8+ messages in thread
From: Rickard Strandqvist @ 2015-01-01 17:02 UTC (permalink / raw)
To: Joshua Thompson, Geert Uytterhoeven
Cc: Rickard Strandqvist, linux-m68k, linux-kernel
Removes some functions that are not used anywhere:
mac_pram_write() mac_pram_read()
This was partially found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
arch/m68k/mac/misc.c | 46 ----------------------------------------------
1 file changed, 46 deletions(-)
diff --git a/arch/m68k/mac/misc.c b/arch/m68k/mac/misc.c
index 707b61a..6fe7eb6 100644
--- a/arch/m68k/mac/misc.c
+++ b/arch/m68k/mac/misc.c
@@ -453,52 +453,6 @@ void pmu_shutdown(void)
*-------------------------------------------------------------------
*/
-void mac_pram_read(int offset, __u8 *buffer, int len)
-{
- __u8 (*func)(int);
- int i;
-
- switch(macintosh_config->adb_type) {
- case MAC_ADB_IISI:
- func = maciisi_read_pram; break;
- case MAC_ADB_PB1:
- case MAC_ADB_PB2:
- func = pmu_read_pram; break;
- case MAC_ADB_CUDA:
- func = cuda_read_pram; break;
- default:
- func = via_read_pram;
- }
- if (!func)
- return;
- for (i = 0 ; i < len ; i++) {
- buffer[i] = (*func)(offset++);
- }
-}
-
-void mac_pram_write(int offset, __u8 *buffer, int len)
-{
- void (*func)(int, __u8);
- int i;
-
- switch(macintosh_config->adb_type) {
- case MAC_ADB_IISI:
- func = maciisi_write_pram; break;
- case MAC_ADB_PB1:
- case MAC_ADB_PB2:
- func = pmu_write_pram; break;
- case MAC_ADB_CUDA:
- func = cuda_write_pram; break;
- default:
- func = via_write_pram;
- }
- if (!func)
- return;
- for (i = 0 ; i < len ; i++) {
- (*func)(offset++, buffer[i]);
- }
-}
-
void mac_poweroff(void)
{
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-01-01 17:02 [PATCH] arch: m68k: mac: misc.c: Remove some unused functions Rickard Strandqvist
@ 2015-01-04 7:21 ` Finn Thain
2015-01-04 19:36 ` Geert Uytterhoeven
0 siblings, 1 reply; 8+ messages in thread
From: Finn Thain @ 2015-01-04 7:21 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Joshua Thompson, Geert Uytterhoeven, linux-m68k, linux-kernel
On Thu, 1 Jan 2015, Rickard Strandqvist wrote:
> Removes some functions that are not used anywhere:
> mac_pram_write() mac_pram_read()
>
If you remove those functions, you'd then find that all of the called
functions become unused:
maciisi_read_pram
maciisi_write_pram
pmu_read_pram
pmu_write_pram
cuda_read_pram
cuda_write_pram
via_read_pram
via_write_pram
I'd rather not remove all of this code. Better to finish the
implementation.
If we provide access to the PRAM (i.e. NVRAM) from userspace then the
EMILE bootloader would be able to assign the boot device, for example.
Within the kernel, the CUDA PRAM functions could be used by valkyriefb to
determine the correct default video mode (see comment in that driver).
Would it be acceptable to utilize drivers/char/generic_nvram.c and
CONFIG_GENERIC_NVRAM? This is the PowerMac PRAM driver but looks generic
enough that it may not need any modification for 68k Macs.
--
> This was partially found by using a static code analysis program called
> cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> arch/m68k/mac/misc.c | 46 ----------------------------------------------
> 1 file changed, 46 deletions(-)
>
> diff --git a/arch/m68k/mac/misc.c b/arch/m68k/mac/misc.c
> index 707b61a..6fe7eb6 100644
> --- a/arch/m68k/mac/misc.c
> +++ b/arch/m68k/mac/misc.c
> @@ -453,52 +453,6 @@ void pmu_shutdown(void)
> *-------------------------------------------------------------------
> */
>
> -void mac_pram_read(int offset, __u8 *buffer, int len)
> -{
> - __u8 (*func)(int);
> - int i;
> -
> - switch(macintosh_config->adb_type) {
> - case MAC_ADB_IISI:
> - func = maciisi_read_pram; break;
> - case MAC_ADB_PB1:
> - case MAC_ADB_PB2:
> - func = pmu_read_pram; break;
> - case MAC_ADB_CUDA:
> - func = cuda_read_pram; break;
> - default:
> - func = via_read_pram;
> - }
> - if (!func)
> - return;
> - for (i = 0 ; i < len ; i++) {
> - buffer[i] = (*func)(offset++);
> - }
> -}
> -
> -void mac_pram_write(int offset, __u8 *buffer, int len)
> -{
> - void (*func)(int, __u8);
> - int i;
> -
> - switch(macintosh_config->adb_type) {
> - case MAC_ADB_IISI:
> - func = maciisi_write_pram; break;
> - case MAC_ADB_PB1:
> - case MAC_ADB_PB2:
> - func = pmu_write_pram; break;
> - case MAC_ADB_CUDA:
> - func = cuda_write_pram; break;
> - default:
> - func = via_write_pram;
> - }
> - if (!func)
> - return;
> - for (i = 0 ; i < len ; i++) {
> - (*func)(offset++, buffer[i]);
> - }
> -}
> -
> void mac_poweroff(void)
> {
> /*
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-01-04 7:21 ` Finn Thain
@ 2015-01-04 19:36 ` Geert Uytterhoeven
2015-02-01 3:39 ` nvram and generic_nvram modules are problematic, was " Finn Thain
0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-01-04 19:36 UTC (permalink / raw)
To: Finn Thain; +Cc: Rickard Strandqvist, Joshua Thompson, linux-m68k, linux-kernel
On Sun, Jan 4, 2015 at 8:21 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> On Thu, 1 Jan 2015, Rickard Strandqvist wrote:
>> Removes some functions that are not used anywhere:
>> mac_pram_write() mac_pram_read()
>
> If you remove those functions, you'd then find that all of the called
> functions become unused:
>
> maciisi_read_pram
> maciisi_write_pram
> pmu_read_pram
> pmu_write_pram
> cuda_read_pram
> cuda_write_pram
> via_read_pram
> via_write_pram
>
> I'd rather not remove all of this code. Better to finish the
> implementation.
Indeed.
> Would it be acceptable to utilize drivers/char/generic_nvram.c and
> CONFIG_GENERIC_NVRAM? This is the PowerMac PRAM driver but looks generic
> enough that it may not need any modification for 68k Macs.
Yes, that would be great.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread
* nvram and generic_nvram modules are problematic, was Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-01-04 19:36 ` Geert Uytterhoeven
@ 2015-02-01 3:39 ` Finn Thain
2015-02-01 8:42 ` Russell King - ARM Linux
2015-02-01 9:11 ` Geert Uytterhoeven
0 siblings, 2 replies; 8+ messages in thread
From: Finn Thain @ 2015-02-01 3:39 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rickard Strandqvist, linux-m68k, linux-kernel, linuxppc-dev,
linux-arm-kernel
On Sun, 4 Jan 2015, Geert Uytterhoeven wrote:
> On Sun, Jan 4, 2015 at 8:21 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> > On Thu, 1 Jan 2015, Rickard Strandqvist wrote:
> > > Removes some functions that are not used anywhere:
> > > mac_pram_write() mac_pram_read()
> >
> > ... I'd rather not remove all of this code. Better to finish the
> > implementation.
>
> Indeed.
>
> > Would it be acceptable to utilize drivers/char/generic_nvram.c and
> > CONFIG_GENERIC_NVRAM? This is the PowerMac PRAM driver but looks
> > generic enough that it may not need any modification for 68k Macs.
>
> Yes, that would be great.
>
Unfortunately, it seems to be unworkable.
The only user of generic_nvram is PPC32 (PPC64 could benefit though).
PPC32 uses the driver by defining both CONFIG_NVRAM and
CONFIG_GENERIC_NVRAM.
I tried to simplify this so that CONFIG_GENERIC_NVRAM would build
drivers/char/generic_nvram, while CONFIG_NVRAM would build
drivers/char/nvram, in order that it would become possible to have a
multi-platform kernel with both, either, or niether.
But that approach brings new problems:
- An m68k multi-platform kernel would need to contain both modules, and
both modules would need MODULE_ALIAS_MISCDEV(NVRAM_MINOR). This isn't
going to work. (It's likely that other platforms might also want to use
generic_nvram and the same problem would apply to x86 and ARM.)
- The misc device code in drivers/char/nvram is duplicated in
generic_nvram. To avoid this duplication, the nvram module could
leverage the generic_nvram module instead. This doesn't work either.
Systems like mine (Gentoo) have "alias char-major-10-144 nvram" in
/etc/modprobe.d/i386.conf, which means that accessing /dev/nvram causes
the wrong module to load.
In the end I concluded that the only plausible "generic" driver is
actually drivers/char/nvram itself. Otherwise it would be under an arch/
directory and not under drivers/.
So the nvram module should be the one with
MODULE_ALIAS_MISCDEV(NVRAM_MAJOR), and it should work on any architecture
that needs to use it. (Sure enough, drivers/char/generic_nvram lacks the
MODULE_ALIAS.)
So I believe that the solution is to eliminate drivers/char/generic_nvram
altogether, and move the architecture-specific code out of
drivers/char/nvram, so the nvram module can be re-used more easily. I
think that PPC32, PPC64 and m68k could readily re-use it.
Drivers themselves all test for CONFIG_NVRAM; never CONFIG_GENERIC_NVRAM.
This is another indication that the generic_nvram driver is surplus to
requirement.
The CONFIG_PROC_FS support (/proc/driver/nvram) in the drivers/char/nvram
module is inherently architecture-specific. I suspect that the
Atari-specific code should move to arch/m68k/atari/ and the x86-specific
code should move to arch/x86/.
I find the ARM support in drivers/char/nvram to be surprising, not to say
questionable. The /proc/driver/nvram implementation, given
defined(__arm__), decodes the NVRAM contents in exactly the same format as
when defined(__i386__) || defined(__x86_64__). Whereas, only MIPS and
PowerPC defconfigs set CONFIG_RTC_DRV_CMOS at all, and without that symbol
the driver will never be built for ARM. This raises the question, does
/proc/driver/nvram do anything useful on any ARM platforms?
Some guidance on this problem would be appreciated; all the approaches I
tried led to unsatisfactory compromises. I don't want to keep re-writing
these patches without a workable plan.
--
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: nvram and generic_nvram modules are problematic, was Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-02-01 3:39 ` nvram and generic_nvram modules are problematic, was " Finn Thain
@ 2015-02-01 8:42 ` Russell King - ARM Linux
2015-02-03 6:11 ` Finn Thain
2015-02-01 9:11 ` Geert Uytterhoeven
1 sibling, 1 reply; 8+ messages in thread
From: Russell King - ARM Linux @ 2015-02-01 8:42 UTC (permalink / raw)
To: Finn Thain
Cc: Geert Uytterhoeven, linux-m68k, linuxppc-dev, linux-kernel,
linux-arm-kernel, Rickard Strandqvist
On Sun, Feb 01, 2015 at 02:39:42PM +1100, Finn Thain wrote:
> I find the ARM support in drivers/char/nvram to be surprising, not to say
> questionable. The /proc/driver/nvram implementation, given
> defined(__arm__), decodes the NVRAM contents in exactly the same format as
> when defined(__i386__) || defined(__x86_64__).
That's because it's used on the Netwinder and EBSA285 platforms, which
are PCI-like, complete with a southbridge which makes them look like a
PC.
> Whereas, only MIPS and
> PowerPC defconfigs set CONFIG_RTC_DRV_CMOS at all, and without that symbol
> the driver will never be built for ARM. This raises the question, does
> /proc/driver/nvram do anything useful on any ARM platforms?
Yes.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: nvram and generic_nvram modules are problematic, was Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-02-01 3:39 ` nvram and generic_nvram modules are problematic, was " Finn Thain
2015-02-01 8:42 ` Russell King - ARM Linux
@ 2015-02-01 9:11 ` Geert Uytterhoeven
2015-02-03 3:22 ` Finn Thain
1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-02-01 9:11 UTC (permalink / raw)
To: Finn Thain
Cc: Rickard Strandqvist, linux-m68k, linux-kernel, linuxppc-dev,
linux-arm-kernel
On Sun, Feb 1, 2015 at 4:39 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> On Sun, 4 Jan 2015, Geert Uytterhoeven wrote:
>
>> On Sun, Jan 4, 2015 at 8:21 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
>> > On Thu, 1 Jan 2015, Rickard Strandqvist wrote:
>> > > Removes some functions that are not used anywhere:
>> > > mac_pram_write() mac_pram_read()
>> >
>> > ... I'd rather not remove all of this code. Better to finish the
>> > implementation.
>>
>> Indeed.
>>
>> > Would it be acceptable to utilize drivers/char/generic_nvram.c and
>> > CONFIG_GENERIC_NVRAM? This is the PowerMac PRAM driver but looks
>> > generic enough that it may not need any modification for 68k Macs.
>>
>> Yes, that would be great.
>>
>
> Unfortunately, it seems to be unworkable.
An alternative could be to just provide an nvram attribute file in sysfs,
like many RTC drivers do.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: nvram and generic_nvram modules are problematic, was Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-02-01 9:11 ` Geert Uytterhoeven
@ 2015-02-03 3:22 ` Finn Thain
0 siblings, 0 replies; 8+ messages in thread
From: Finn Thain @ 2015-02-03 3:22 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rickard Strandqvist, linux-m68k, linux-kernel, linuxppc-dev,
linux-arm-kernel
On Sun, 1 Feb 2015, Geert Uytterhoeven wrote:
> On Sun, Feb 1, 2015 at 4:39 AM, Finn Thain wrote:
> > On Sun, 4 Jan 2015, Geert Uytterhoeven wrote:
> > > On Sun, Jan 4, 2015 at 8:21 AM, Finn Thain wrote:
> > > > On Thu, 1 Jan 2015, Rickard Strandqvist wrote:
> > > > > Removes some functions that are not used anywhere:
> > > > > mac_pram_write() mac_pram_read()
> > > >
> > > > ... I'd rather not remove all of this code. Better to finish the
> > > > implementation.
> > >
> > > Indeed.
> > >
> > > > Would it be acceptable to utilize drivers/char/generic_nvram.c and
> > > > CONFIG_GENERIC_NVRAM? This is the PowerMac PRAM driver but looks
> > > > generic enough that it may not need any modification for 68k Macs.
> > >
> > > Yes, that would be great.
> > >
> >
> > Unfortunately, it seems to be unworkable.
>
> An alternative could be to just provide an nvram attribute file in
> sysfs, like many RTC drivers do.
>
Are attribute files seekable? Even if userspace could use "/dev/nvram" and
"/sys/whatever/nvram" interchangably, wouldn't it be better if PPC Macs
and 68k Macs offered a consistent API to userspace?
And your suggestion doesn't solve the problem, that is, to be able to
build a multi-platform kernel binary in which drivers can access NVRAM.
The __nvram_read_byte(), nvram_read_byte() etc functions defined in
drivers/char/nvram.c, if allowed to proliferate because random
architectures might like to use a generic /dev/nvram API, would further
uglify that file.
If the m68k Mac kernel doesn't define the nvram_read_byte() routine then
valkyriefb can't use it. (fbdev drivers are apparently the reason why
powerpc defines them.)
drivers/char/nvram.c has two sets of these routines for PC RTC NVRAM; one
for m68k (Atari) and one for arm/x86. We don't want to introduce more code
into drivers/char/nvram.c to support all four configurations:
1) arm/x86
2) atari
3) atari + mac
4) mac
So we'd end up having to move m68k-specific code out of
drivers/char/nvram.c, to make it generic. And that then begs all of the
questions in my previous message.
BTW, my experimental patches replaced all of those __nvram_* and nvram_*
functions with an ops struct. E.g.
$ cat include/linux/nvram.h
#ifndef _LINUX_NVRAM_H
#define _LINUX_NVRAM_H
#include <uapi/linux/nvram.h>
struct nvram_ops {
ssize_t (*read)(char *, size_t, loff_t *);
ssize_t (*write)(char *, size_t, loff_t *);
unsigned char (*read_byte)(int);
void (*write_byte)(unsigned char, int);
ssize_t (*get_size)(void);
#ifdef CONFIG_PPC
void (*sync)(void);
#else
long (*set_checksum)(void);
long (*initialize)(void);
#endif
};
extern const struct nvram_ops arch_nvram_ops;
extern const struct nvram_ops rtc_nvram_ops;
#endif /* _LINUX_NVRAM_H */
This experiment has m68k implement arch_nvram_ops that dispatch to Atari
or Mac methods (at compile-time for a single-platform kernel, or at
run-time for a multi-platform kernel binary).
But this implies modifications to fbdev drivers, PPC32 and PPC64, nvram
and generic_nvram modules. And any work at all done on generic_nvram seems
to be misguided, unless it is removal.
--
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: nvram and generic_nvram modules are problematic, was Re: [PATCH] arch: m68k: mac: misc.c: Remove some unused functions
2015-02-01 8:42 ` Russell King - ARM Linux
@ 2015-02-03 6:11 ` Finn Thain
0 siblings, 0 replies; 8+ messages in thread
From: Finn Thain @ 2015-02-03 6:11 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Geert Uytterhoeven, linux-m68k, linuxppc-dev, linux-kernel,
linux-arm-kernel, Rickard Strandqvist
On Sun, 1 Feb 2015, Russell King - ARM Linux wrote:
> On Sun, Feb 01, 2015 at 02:39:42PM +1100, Finn Thain wrote:
> > I find the ARM support in drivers/char/nvram to be surprising, not to
> > say questionable. The /proc/driver/nvram implementation, given
> > defined(__arm__), decodes the NVRAM contents in exactly the same
> > format as when defined(__i386__) || defined(__x86_64__).
>
> That's because it's used on the Netwinder and EBSA285 platforms, which
> are PCI-like, complete with a southbridge which makes them look like a
> PC.
Well, that explains the presence of RTC NVRAM, but the question was also
about the format of that NVRAM.
The the code in question in drivers/char/nvram.c decodes RTC NVRAM on ARM
as if it took the same format as x86 PC RTC NVRAM. Apparently they are not
in the same format (for EBSA-285 at least).
I downloaded the EBSA-285 BIOS,
ftp://ftp.arm.linux.org.uk/pub/armlinux/source/boot/bios-1.11.tar.gz
This is from bios/init/cfg.c --
img_nr = rtc_read_cmos(0);
root_dev = rtc_read_cmos(2) | rtc_read_cmos(3) << 8;
for (i = 0; i < 80; i++) {
extra_args[i] = rtc_read_cmos(128+i);
if (!extra_args[i])
break;
}
extra_args[79] = '\0';
(Like /dev/nvram, file offset 0 is actually device offset 14.)
This EBSA-285 BIOS code uses bytes 128 through 208 as "extra_args",
whereas /dev/nvram has only 114 bytes in total. The EBSA-285 BIOS doesn't
read or write any checksum.
The first two bytes aren't reported in /proc/driver/nvram, whereas
EBSA-285 BIOS uses them for img_nr and root_dev. The next one is reported
as "floppy type" on x86 PC, though the EBSA-285 BIOS uses it for
root_dev >> 8.
This is from an x86 PC, for example:
# cat /proc/driver/nvram
Checksum status: valid
# floppies : 1
Floppy 0 type : none
Floppy 1 type : none
HD 0 type : 01
HD 1 type : none
HD type 48 data: 512/0/0 C/H/S, precomp 0, lz 256
HD type 49 data: 1/124/0 C/H/S, precomp 0, lz 0
DOS base memory: 640 kB
Extended memory: 64512 kB (configured), 64512 kB (tested)
Gfx adapter : EGA, VGA, ... (with BIOS)
FPU : installed
My original question was whether the CONFIG_PROC_FS support found in
drivers/char/nvram.c should be moved to arch/x86 and arch/m68k.
So that question now seems to hinge on the Netwinder ROM ("nettrom") which
I gather is proprietary (I didn't find any source code). Does anyone know
what format the Netwinder NVRAM takes?
If NVRAM on Netwinder has a different format to x86 PC, then the
CONFIG_PROC_FS code in drivers/char/nvram should go elsewhere.
And if the NVRAM on Netwinder had no checksum, drivers/char/nvram.c would
become a generic /dev/nvram misc device, and drivers/char/generic_nvram.c
could go away, along with its inherent problems.
--
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-02-03 6:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-01 17:02 [PATCH] arch: m68k: mac: misc.c: Remove some unused functions Rickard Strandqvist
2015-01-04 7:21 ` Finn Thain
2015-01-04 19:36 ` Geert Uytterhoeven
2015-02-01 3:39 ` nvram and generic_nvram modules are problematic, was " Finn Thain
2015-02-01 8:42 ` Russell King - ARM Linux
2015-02-03 6:11 ` Finn Thain
2015-02-01 9:11 ` Geert Uytterhoeven
2015-02-03 3:22 ` Finn Thain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).