Hello,
ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try to
load it on my laptop, I get ENODEV. This is caused by platform_driver_probe()
- the code never reaches the ramoops probe callback.
After I removed the platform_driver stuff, moving everything into ramoops_init
and ramoops_exit it worked.
On Mon, May 23, 2011 at 9:27 PM, Stevie Trujillo
<[email protected]> wrote:
> Hello,
>
> ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try to
> load it on my laptop, I get ENODEV. This is caused by platform_driver_probe()
> - the code never reaches the ramoops probe callback.
>
> After I removed the platform_driver stuff, moving everything into ramoops_init
> and ramoops_exit it worked.
Actually that was changed by Kyungmin, Cc'ing...
commit c3b92ce9e75f6353104fc7f8e32fb9fdb2550ad0
Author: Kyungmin Park <[email protected]>
Date: Wed Oct 27 15:34:52 2010 -0700
ramoops: use the platform data structure instead of module params
As each board and system has different memory for ramoops. It's better to
define the platform data instead of module params.
[[email protected]: fix ramoops_remove() return type]
Signed-off-by: Kyungmin Park <[email protected]>
Cc: Marco Stornelli <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Hi,
You have to define the ramoops platform data at your board file and
pass it to the platform device init.
As these address is different for each SoCs. e.g., x86, and Samsung
ARM SoCs and so on.
I think maybe you use the x86 so define the default x86 ram address
for ramoops and pass it to platform structures.
At office, I will send the sample usage.
Thank you,
Kyungmin Park
On Mon, May 23, 2011 at 11:10 PM, Am?rico Wang <[email protected]> wrote:
> On Mon, May 23, 2011 at 9:27 PM, Stevie Trujillo
> <[email protected]> wrote:
>> Hello,
>>
>> ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try to
>> load it on my laptop, I get ENODEV. This is caused by platform_driver_probe()
>> - the code never reaches the ramoops probe callback.
>>
>> After I removed the platform_driver stuff, moving everything into ramoops_init
>> and ramoops_exit it worked.
>
> Actually that was changed by Kyungmin, Cc'ing...
>
> commit c3b92ce9e75f6353104fc7f8e32fb9fdb2550ad0
> Author: Kyungmin Park <[email protected]>
> Date: ? Wed Oct 27 15:34:52 2010 -0700
>
> ? ?ramoops: use the platform data structure instead of module params
>
> ? ?As each board and system has different memory for ramoops. ?It's better to
> ? ?define the platform data instead of module params.
>
> ? ?[[email protected]: fix ramoops_remove() return type]
> ? ?Signed-off-by: Kyungmin Park <[email protected]>
> ? ?Cc: Marco Stornelli <[email protected]>
> ? ?Signed-off-by: Andrew Morton <[email protected]>
> ? ?Signed-off-by: Linus Torvalds <[email protected]>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
On Mon, May 23, 2011 at 11:36 PM, Kyungmin Park <[email protected]> wrote:
> Hi,
>
> You have to define the ramoops platform data at your board file and
> pass it to the platform device init.
> As these address is different for each SoCs. e.g., x86, and Samsung
> ARM SoCs and so on.
>
> I think maybe you use the x86 so define the default x86 ram address
> for ramoops and pass it to platform structures.
>
> At office, I will send the sample usage.
+static struct ramoops_platform_data goni_ramoops_data = {
+ .mem_size = SZ_16K,
+ .mem_address = 0xED000000, /* SRAM */
+};
+
+static struct platform_device goni_ramoops = {
+ .name = "ramoops",
+ .dev = {
+ .platform_data = &goni_ramoops_data,
+ },
+};
and register the goni_rammoops. then you can find a rammops.
Thank you,
Kyungmin Park
>
> Thank you,
> Kyungmin Park
>
> On Mon, May 23, 2011 at 11:10 PM, Am?rico Wang <[email protected]> wrote:
>> On Mon, May 23, 2011 at 9:27 PM, Stevie Trujillo
>> <[email protected]> wrote:
>>> Hello,
>>>
>>> ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try to
>>> load it on my laptop, I get ENODEV. This is caused by platform_driver_probe()
>>> - the code never reaches the ramoops probe callback.
>>>
>>> After I removed the platform_driver stuff, moving everything into ramoops_init
>>> and ramoops_exit it worked.
>>
>> Actually that was changed by Kyungmin, Cc'ing...
>>
>> commit c3b92ce9e75f6353104fc7f8e32fb9fdb2550ad0
>> Author: Kyungmin Park <[email protected]>
>> Date: ? Wed Oct 27 15:34:52 2010 -0700
>>
>> ? ?ramoops: use the platform data structure instead of module params
>>
>> ? ?As each board and system has different memory for ramoops. ?It's better to
>> ? ?define the platform data instead of module params.
>>
>> ? ?[[email protected]: fix ramoops_remove() return type]
>> ? ?Signed-off-by: Kyungmin Park <[email protected]>
>> ? ?Cc: Marco Stornelli <[email protected]>
>> ? ?Signed-off-by: Andrew Morton <[email protected]>
>> ? ?Signed-off-by: Linus Torvalds <[email protected]>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at ?http://www.tux.org/lkml/
>>
>
On Tue, May 24, 2011 at 1:32 PM, Kyungmin Park <[email protected]> wrote:
> On Mon, May 23, 2011 at 11:36 PM, Kyungmin Park <[email protected]> wrote:
>> Hi,
>>
>> You have to define the ramoops platform data at your board file and
>> pass it to the platform device init.
>> As these address is different for each SoCs. e.g., x86, and Samsung
>> ARM SoCs and so on.
>>
>> I think maybe you use the x86 so define the default x86 ram address
>> for ramoops and pass it to platform structures.
Why not document this?
>>
>> At office, I will send the sample usage.
>
> +static struct ramoops_platform_data goni_ramoops_data = {
> + .mem_size = SZ_16K,
> + .mem_address = 0xED000000, /* SRAM */
> +};
> +
> +static struct platform_device goni_ramoops = {
> + .name = "ramoops",
> + .dev = {
> + .platform_data = &goni_ramoops_data,
> + },
> +};
>
> and register the goni_rammoops. then you can find a rammops.
>
Huh? Is this for x86 too? Why so unfriendly for end-users?
I think we need some kernel parameter like 'crashkernel=' (or memmap=)
to reserve memory for ramoops, right?
Thanks.
On Tue, May 24, 2011 at 2:49 PM, Am?rico Wang <[email protected]> wrote:
> On Tue, May 24, 2011 at 1:32 PM, Kyungmin Park <[email protected]> wrote:
>> On Mon, May 23, 2011 at 11:36 PM, Kyungmin Park <[email protected]> wrote:
>>> Hi,
>>>
>>> You have to define the ramoops platform data at your board file and
>>> pass it to the platform device init.
>>> As these address is different for each SoCs. e.g., x86, and Samsung
>>> ARM SoCs and so on.
>>>
>>> I think maybe you use the x86 so define the default x86 ram address
>>> for ramoops and pass it to platform structures.
>
> Why not document this?
>
>>>
>>> At office, I will send the sample usage.
>>
>> +static struct ramoops_platform_data goni_ramoops_data = {
>> + ? ? ? .mem_size ? ? ? ? ? ? ? = SZ_16K,
>> + ? ? ? .mem_address ? ? ? ? ? ?= 0xED000000, ? /* SRAM */
>> +};
>> +
>> +static struct platform_device goni_ramoops = {
>> + ? ? ? .name = "ramoops",
>> + ? ? ? .dev = {
>> + ? ? ? ? ? ? ? .platform_data = &goni_ramoops_data,
>> + ? ? ? },
>> +};
>>
>> and register the goni_rammoops. then you can find a rammops.
>>
>
> Huh? Is this for x86 too? Why so unfriendly for end-users?
I don't know which address is acceptable for x86, in case of ARM, each
SoCs has different SRAM address. so it's not good to define for all
SoCs and ARM.
>
> I think we need some kernel parameter like 'crashkernel=' (or memmap=)
> to reserve memory for ramoops, right?
The first implementation is just module parameters.
ramoops.address=0x??????? ramoops.size=0x????. So I patched it as
using platform devices.
and the reason use the platform is it's dependent on each SoCs and board usage.
>
> Thanks.
>
On Tue, May 24, 2011 at 2:14 PM, Kyungmin Park <[email protected]> wrote:
> On Tue, May 24, 2011 at 2:49 PM, Américo Wang <[email protected]> wrote:
>> Huh? Is this for x86 too? Why so unfriendly for end-users?
> I don't know which address is acceptable for x86, in case of ARM, each
> SoCs has different SRAM address. so it's not good to define for all
> SoCs and ARM.
>>
>> I think we need some kernel parameter like 'crashkernel=' (or memmap=)
>> to reserve memory for ramoops, right?
>
> The first implementation is just module parameters.
> ramoops.address=0x??????? ramoops.size=0x????. So I patched it as
> using platform devices.
> and the reason use the platform is it's dependent on each SoCs and board usage.
>
But the result is that this makes end-users harder to use it.
Using platform API still relies on a hard-code address, at least in
your example,
so, why not leave it as a module parameter to let user to find the
correct address?
On Tue, May 24, 2011 at 11:12 PM, Am?rico Wang <[email protected]> wrote:
> On Tue, May 24, 2011 at 2:14 PM, Kyungmin Park <[email protected]> wrote:
>> On Tue, May 24, 2011 at 2:49 PM, Am?rico Wang <[email protected]> wrote:
>>> Huh? Is this for x86 too? Why so unfriendly for end-users?
>> I don't know which address is acceptable for x86, in case of ARM, each
>> SoCs has different SRAM address. so it's not good to define for all
>> SoCs and ARM.
>>>
>>> I think we need some kernel parameter like 'crashkernel=' (or memmap=)
>>> to reserve memory for ramoops, right?
>>
>> The first implementation is just module parameters.
>> ramoops.address=0x??????? ramoops.size=0x????. So I patched it as
>> using platform devices.
>> and the reason use the platform is it's dependent on each SoCs and board usage.
>>
>
> But the result is that this makes end-users harder to use it.
>
> Using platform API still relies on a hard-code address, at least in
> your example,
> so, why not leave it as a module parameter to let user to find the
> correct address?
It's possible. I just make it possible to use the platform driver. you
can specify the original method.
Thank you,
Kyungmin Park
>
On Tuesday 24 May 2011 16:16:13 Kyungmin Park wrote:
> On Tue, May 24, 2011 at 11:12 PM, Am?rico Wang <[email protected]>
wrote:
> > On Tue, May 24, 2011 at 2:14 PM, Kyungmin Park <[email protected]>
wrote:
> >> On Tue, May 24, 2011 at 2:49 PM, Am?rico Wang <[email protected]>
wrote:
> >>> Huh? Is this for x86 too? Why so unfriendly for end-users?
> >>
> >> I don't know which address is acceptable for x86, in case of ARM, each
> >> SoCs has different SRAM address. so it's not good to define for all
> >> SoCs and ARM.
> >>
> >>> I think we need some kernel parameter like 'crashkernel=' (or memmap=)
> >>> to reserve memory for ramoops, right?
> >>
> >> The first implementation is just module parameters.
> >> ramoops.address=0x??????? ramoops.size=0x????. So I patched it as
> >> using platform devices.
> >> and the reason use the platform is it's dependent on each SoCs and board
> >> usage.
> >
> > But the result is that this makes end-users harder to use it.
> >
> > Using platform API still relies on a hard-code address, at least in
> > your example,
> > so, why not leave it as a module parameter to let user to find the
> > correct address?
>
> It's possible. I just make it possible to use the platform driver. you
> can specify the original method.
I don't think it's possible without also having a platform_device.
ramoops_probe is never called here, I think platform_driver_probe returns -
ENODEV.
static int __init ramoops_init(void)
{
return platform_driver_probe(&ramoops_driver, ramoops_probe);
}
Maybe one could run platform_driver_probe() only if (!mem_address &&
!mem_size)?
Unrelated question: should the printk()s end with "\n"? I see they do that
other places in the kernel.
Il 23/05/2011 15:27, Stevie Trujillo ha scritto:
> Hello,
>
> ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try to
Yes, or at least was my intention. It's true that this drivers is useful
when you use an NVRAM, this is typical for the embedded world where the
platform driver approach is more diffused. Sure at this point the module
parameters are not useful. In addition the platform data struct doesn't
define a way to select if to dump only oops. At the end I think a patch
it's needed here. I have to look at the code to see if it's possible to
use the platform data OR module parameters. I'll submit a patch.
Marco
On Wed, May 25, 2011 at 1:03 AM, Marco Stornelli
<[email protected]> wrote:
> Il 23/05/2011 15:27, Stevie Trujillo ha scritto:
>>
>> Hello,
>>
>> ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try
>> to
>
> Yes, or at least was my intention. It's true that this drivers is useful
> when you use an NVRAM, this is typical for the embedded world where the
> platform driver approach is more diffused. Sure at this point the module
> parameters are not useful. In addition the platform data struct doesn't
> define a way to select if to dump only oops. At the end I think a patch it's
> needed here. I have to look at the code to see if it's possible to use the
> platform data OR module parameters. I'll submit a patch.
>
But we have the following code:
if (reason != KMSG_DUMP_OOPS &&
reason != KMSG_DUMP_PANIC &&
reason != KMSG_DUMP_KEXEC)
return;
which is what you meant by saying "only oops" ?
I still don't think that is a correct way to tell people not to use
ramoops, we need to document this rather than prevent using
it in the code.
Thanks.
2011/5/25 Am?rico Wang <[email protected]>:
> On Wed, May 25, 2011 at 1:03 AM, Marco Stornelli
> <[email protected]> wrote:
>> Il 23/05/2011 15:27, Stevie Trujillo ha scritto:
>>>
>>> Hello,
>>>
>>> ramoops (drivers/char/ramoops.c) is for "all" computers right? When I try
>>> to
>>
>> Yes, or at least was my intention. It's true that this drivers is useful
>> when you use an NVRAM, this is typical for the embedded world where the
>> platform driver approach is more diffused. Sure at this point the module
>> parameters are not useful. In addition the platform data struct doesn't
>> define a way to select if to dump only oops. At the end I think a patch it's
>> needed here. I have to look at the code to see if it's possible to use the
>> platform data OR module parameters. I'll submit a patch.
>>
>
> But we have the following code:
>
> ? ? ? ?if (reason != KMSG_DUMP_OOPS &&
> ? ? ? ? ? ?reason != KMSG_DUMP_PANIC &&
> ? ? ? ? ? ?reason != KMSG_DUMP_KEXEC)
> ? ? ? ? ? ? ? ?return;
>
> which is what you meant by saying "only oops" ?
I meant a way to set dump_oops parameter via platform data.
>
> I still don't think that is a correct way to tell people not to use
> ramoops, we need to document this rather than prevent using
> it in the code.
>
I really don't understand what you mean here.
Marco