Re: [Qemu-trivial] [PATCH 10/11] arm: add missing static and remove unused functions

[Peter Maydell]

On 14 October 2012 20:58, Blue Swirl <address@hidden> wrote:
> index 2fc4137..2c02a83 100644
> --- a/hw/omap_gpmc.c
> +++ b/hw/omap_gpmc.c
> @@ -871,24 +871,3 @@ void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, 
> MemoryRegion *iomem)
>      f->iomem = iomem;
>      omap_gpmc_cs_map(s, cs);
>  }
> -
> -void omap_gpmc_attach_nand(struct omap_gpmc_s *s, int cs, DeviceState *nand)
> -{
> -    struct omap_gpmc_cs_file_s *f;
> -    assert(nand);
> -
> -    if (cs < 0 || cs >= 8) {
> -        fprintf(stderr, "%s: bad chip-select %i\n", __func__, cs);
> -        exit(-1);
> -    }
> -    f = &s->cs_file[cs];
> -
> -    omap_gpmc_cs_unmap(s, cs);
> -    f->config[0] &= ~(0xf << 10);
> -    f->config[0] |= (OMAP_GPMC_NAND << 10);
> -    f->dev = nand;
> -    if (nand_getbuswidth(f->dev) == 16) {
> -        f->config[0] |= OMAP_GPMC_16BIT << 12;
> -    }
> -    omap_gpmc_cs_map(s, cs);
> -}

Please don't delete this function, it is the public facing interface
for allowing board models to attach NAND devices to the GPMC. This
might not be used by anything currently in mainline, but it is used
by the omap3 beagle and overo board models in qemu-linaro (and which
I will upstream eventually, honest).

More generally I'm wary of deletion of apparently unused functions like
this one (or some of the others like the 'pcmcia eject card' function)
which are obviously intended to be public facing APIs to other
device models, unless they come with rationales along the lines of
"this function was added for purpose X but it is not needed now because
commit Y changed things so you do this with Z now".

> diff --git a/linux-user/arm/nwfpe/fpa11.c b/linux-user/arm/nwfpe/fpa11.c
> index eebd93f..3434036 100644
> --- a/linux-user/arm/nwfpe/fpa11.c
> +++ b/linux-user/arm/nwfpe/fpa11.c

In general anything in linux-user/arm/nwfpe is legacy code which
it's scarcely worth the effort of touching or reviewing.

thanks
-- PMM