| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 30 Apr 2007 23:56:30 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Paul Sokolovsky <pmiscml@...il.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [RFC, PATCH 1/4] SoC base drivers: SoC helper API
On Tue, 1 May 2007 08:08:27 +0300 Paul Sokolovsky <pmiscml@...il.com> wrote:
> diff --git a/drivers/soc/soc-core.c b/drivers/soc/soc-core.c
> new file mode 100644
> index 0000000..24c654c
> --- /dev/null
> +++ b/drivers/soc/soc-core.c
> @@ -0,0 +1,106 @@
> +/*
> + * drivers/soc/soc-core.c
> + *
> + * core SoC support
> + * Copyright (c) 2006 Ian Molton
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This file contains functionality used by many SoC type devices.
> + *
> + * Created: 2006-11-28
> + *
> + */
> +
> +#include <linux/ioport.h>
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include "soc-core.h"
> +
> +void soc_free_devices(struct platform_device *devices, int nr_devs)
> +{
> + struct platform_device *dev = devices;
> + int i;
> +
> + for (i = 0; i < nr_devs; i++) {
> + struct resource *res = dev->resource;
> + platform_device_unregister(dev++);
> + kfree(res);
> + }
> + kfree(devices);
> +}
> +EXPORT_SYMBOL_GPL(soc_free_devices);
> +
> +#define SIGNED_SHIFT(val, shift) ((shift) >= 0 ? ((val) << (shift)) : ((val) >> -(shift)))
This will do bad things with
SIGNED_SHIFT(val, distance++);
So either
a) take a copy of the args inside the macro or, better
b) use a static inline.
It's probably worth putting the result into include/linux/kernel.h as well.
It's quite generic. If so, that's a standalone patch, please. Or keep it
here if you can't be bothered with that.
> +struct platform_device *soc_add_devices(struct platform_device *dev,
> + struct soc_device_data *soc, int nr_devs,
> + struct resource *mem,
> + int relative_addr_shift, int irq_base)
> +{
> + struct platform_device *devices;
> + int i, r, base;
> +
> + devices = kzalloc(nr_devs * sizeof(struct platform_device), GFP_KERNEL);
> + if (!devices)
> + return NULL;
> +
> + for (i = 0; i < nr_devs; i++) {
> + struct platform_device *sdev = &devices[i];
> + struct soc_device_data *blk = &soc[i];
> + struct resource *res;
> +
> + sdev->id = -1;
> + sdev->name = blk->name;
> +
> + sdev->dev.parent = &dev->dev;
> + sdev->dev.platform_data = (void *)blk->hwconfig;
> + sdev->num_resources = blk->num_resources;
> +
> + /* Allocate space for the subdevice resources */
> + res = kzalloc (blk->num_resources * sizeof (struct resource), GFP_KERNEL);
> + if (!res)
> + goto fail;
> +
> + for (r = 0 ; r < blk->num_resources ; r++) {
> + res[r].name = blk->res[r].name; // Fixme - should copy
fix me then ;)
With kstrdup(), presumably.
> +
> + /* Find out base to use */
> + base = 0;
> + if (blk->res[r].flags & IORESOURCE_MEM) {
> + base = mem->start;
mem->start is of type resource_size_t, and `base' should be of that type as
well.
> + } else if ((blk->res[r].flags & IORESOURCE_IRQ) &&
> + (blk->res[r].flags & IORESOURCE_IRQ_SOC_SUBDEVICE)) {
> + base = irq_base;
> + }
> +
> + /* Adjust resource */
> + if (blk->res[r].flags & IORESOURCE_MEM) {
> + res[r].parent = mem;
> + res[r].start = base + SIGNED_SHIFT(blk->res[r].start, relative_addr_shift);
> + res[r].end = base + SIGNED_SHIFT(blk->res[r].end, relative_addr_shift);
Which means that SIGNED_SHIFT really has to be a macro, as it needs to
handle resource_size_t's and you don't know what type they have.
Also, things get interesting when working out how to handle right-shift of
a negative quantity of unknown type.
> + } else {
> + res[r].start = base + blk->res[r].start;
> + res[r].end = base + blk->res[r].end;
> + }
> + res[r].flags = blk->res[r].flags;
> + }
> +
> + sdev->resource = res;
> + if (platform_device_register(sdev)) {
> + kfree(res);
> + goto fail;
> + }
> +
> + printk(KERN_INFO "SoC: registering %s\n", blk->name);
> + }
> + return devices;
> +
> +fail:
> + soc_free_devices(devices, i + 1);
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(soc_add_devices);
> diff --git a/drivers/soc/soc-core.h b/drivers/soc/soc-core.h
> new file mode 100644
> index 0000000..8659f7e
> --- /dev/null
> +++ b/drivers/soc/soc-core.h
> @@ -0,0 +1,30 @@
> +/*
> + * drivers/soc/soc-core.h
> + *
> + * core SoC support
> + * Copyright (c) 2006 Ian Molton
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This file contains prototypes for the functions in soc-core.c
> + *
> + * Created: 2006-11-28
> + *
> + */
> +
> +struct soc_device_data {
> + char *name;
> + struct resource *res;
> + int num_resources;
> + void *hwconfig; /* platform_data to pass to the subdevice */
> +};
> +
> +struct platform_device *soc_add_devices(struct platform_device *dev,
> + struct soc_device_data *soc, int n_devs,
> + struct resource *mem,
> + int relative_addr_shift, int irq_base);
> +
> +void soc_free_devices(struct platform_device *devices, int nr_devs);
> +
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 6859a3b..65f0fcd 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -57,6 +57,9 @@ struct resource_list {
> #define IORESOURCE_IRQ_LOWLEVEL (1<<3)
> #define IORESOURCE_IRQ_SHAREABLE (1<<4)
>
> +/* SoC device IRQ specific bits (IORESOURCE_BITS) */
> +#define IORESOURCE_IRQ_SOC_SUBDEVICE (1<<5)
> +
> /* ISA PnP DMA specific bits (IORESOURCE_BITS) */
> #define IORESOURCE_DMA_TYPE_MASK (3<<0)
> #define IORESOURCE_DMA_8BIT (0<<0)
>
It looks reasonable to me, but I'm not Greg or Russell or anything like
that.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists