Re: [PATCH 1/2 v2] ramoops: use module parameters instead of platformdata if not available

[AmÃrico Wang]

On Sat, May 28, 2011 at 10:48 PM, Marco Stornelli
<marco.stornelli@xxxxxxxxx> wrote:
> From: Marco Stornelli <marco.stornelli@xxxxxxxxx>
>
> Use generic module parameters instead of platform data, if platform
> data are not available. This limitation has been introduced with
> commit c3b92ce9e75f6353104fc7f8e32fb9fdb2550ad0.
>
> Signed-off-by: Marco Stornelli <marco.stornelli@xxxxxxxxx>
> CC: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> Reported-by: Stevie Trujillo <stevie.trujillo@xxxxxxxxx>
> ---
> ChangeLog
> v2: fixed tab/space problem
>
> diff --git a/drivers/char/ramoops.c b/drivers/char/ramoops.c
> index 1a9f5f6..bf5f9f6 100644
> --- a/drivers/char/ramoops.c
> +++ b/drivers/char/ramoops.c
> @@ -26,6 +26,7 @@
> Â#include <linux/io.h>
> Â#include <linux/ioport.h>
> Â#include <linux/platform_device.h>
> +#include <linux/slab.h>
> Â#include <linux/ramoops.h>
>
> Â#define RAMOOPS_KERNMSG_HDR "===="
> @@ -56,6 +57,9 @@ static struct ramoops_context {
> Â Â Â Âint max_count;
> Â} oops_cxt;
>
> +static struct platform_device *dummy;
> +static struct ramoops_platform_data *dummy_data;
> +
> Âstatic void ramoops_do_dump(struct kmsg_dumper *dumper,
> Â Â Â Â Â Â Â Âenum kmsg_dump_reason reason, const char *s1, unsigned long l1,
> Â Â Â Â Â Â Â Âconst char *s2, unsigned long l2)
> @@ -106,27 +110,22 @@ static int __init ramoops_probe(struct platform_device *pdev)
> Â Â Â Âstruct ramoops_context *cxt = &oops_cxt;
> Â Â Â Âint err = -EINVAL;
>
> - Â Â Â if (pdata) {
> - Â Â Â Â Â Â Â mem_size = pdata->mem_size;
> - Â Â Â Â Â Â Â mem_address = pdata->mem_address;
> - Â Â Â }
> -
> - Â Â Â if (!mem_size) {
> + Â Â Â if (!pdata->mem_size) {
> Â Â Â Â Â Â Â Âprintk(KERN_ERR "ramoops: invalid size specification");
> Â Â Â Â Â Â Â Âgoto fail3;
> Â Â Â Â}
>
> - Â Â Â rounddown_pow_of_two(mem_size);
> + Â Â Â rounddown_pow_of_two(pdata->mem_size);
>
> - Â Â Â if (mem_size < RECORD_SIZE) {
> + Â Â Â if (pdata->mem_size < RECORD_SIZE) {
> Â Â Â Â Â Â Â Âprintk(KERN_ERR "ramoops: size too small");
> Â Â Â Â Â Â Â Âgoto fail3;
> Â Â Â Â}
>
> - Â Â Â cxt->max_count = mem_size / RECORD_SIZE;
> + Â Â Â cxt->max_count = pdata->mem_size / RECORD_SIZE;
> Â Â Â Âcxt->count = 0;
> - Â Â Â cxt->size = mem_size;
> - Â Â Â cxt->phys_addr = mem_address;
> + Â Â Â cxt->size = pdata->mem_size;
> + Â Â Â cxt->phys_addr = pdata->mem_address;
>
> Â Â Â Âif (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
> Â Â Â Â Â Â Â Âprintk(KERN_ERR "ramoops: request mem region failed");
> @@ -179,12 +178,35 @@ static struct platform_driver ramoops_driver = {
>
> Âstatic int __init ramoops_init(void)
> Â{
> - Â Â Â return platform_driver_probe(&ramoops_driver, ramoops_probe);
> + Â Â Â int ret;
> + Â Â Â ret = platform_driver_probe(&ramoops_driver, ramoops_probe);
> + Â Â Â if (ret == -ENODEV)
> + Â Â Â {


Wrong coding style...

> + Â Â Â Â Â Â Â /*
> + Â Â Â Â Â Â Â Â* if we didn't find a platform device, we use module parameters
> + Â Â Â Â Â Â Â Â* building platform data on the fly.
> + Â Â Â Â Â Â Â Â*/
> + Â Â Â Â Â Â Â dummy_data = (struct ramoops_platform_data *)
> + Â Â Â Â Â Â Â Â Â Âkzalloc(sizeof(struct ramoops_platform_data), GFP_KERNEL);

Please check the return value of kzalloc(), and, you don't need to
cast it.

> + Â Â Â Â Â Â Â dummy_data->mem_size = mem_size;
> + Â Â Â Â Â Â Â dummy_data->mem_address = mem_address;
> + Â Â Â Â Â Â Â dummy = platform_create_bundle(&ramoops_driver, ramoops_probe,
> + Â Â Â Â Â Â Â Â Â Â Â NULL, 0, dummy_data,
> + Â Â Â Â Â Â Â Â Â Â Â sizeof(struct ramoops_platform_data));
> +
> + Â Â Â Â Â Â Â if (IS_ERR(dummy))
> + Â Â Â Â Â Â Â Â Â Â Â ret = PTR_ERR(dummy);
> + Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â ret = 0;
> + Â Â Â }
> +
> + Â Â Â return ret;
> Â}
>
> Âstatic void __exit ramoops_exit(void)
> Â{
> Â Â Â Âplatform_driver_unregister(&ramoops_driver);
> + Â Â Â kfree(dummy_data);
> Â}
>
> Âmodule_init(ramoops_init);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/