> The driver will be probed for all devices with the type ipmi. It's
> supporting
> devices with compatible settings ipmi-kcs, ipmi-smic and ipmi-bt.
> Only ipmi-kcs could be tested.
I'd only include that one then. But the code is trivial,
the risk is minimal, why not.
> +/* only exists on powerpc */
> +#ifdef CONFIG_PPC_OF
> +#include <asm/of_device.h>
> +#include <asm/of_platform.h>
> +#endif
Comment is inexact -- just kill it.
> +static int ipmi_of_probe(struct of_device *dev,
> + const struct of_device_id *match)
Shouldn't this (and everything else) be some kind of __init?
> + regsize = get_property(np, "reg-size", NULL);
> + regspacing = get_property(np, "reg-spacing", NULL);
> + regshift = get_property(np, "reg-shift", NULL);
You should check whether you get exactly 4 bytes back or not.
> + info->si_type = (enum si_type) match->data;
That lands you a compiler warning (cast from pointer to shorter
integer) on 64-bit.
> + if (!regsize)
> + info->io.regsize = DEFAULT_REGSPACING;
> + else
> + info->io.regsize = *regsize;
info->io_regsize = regsize ? *regsize : DEFAULT_REGSIZE;
[Please note that fixes a copy/paste bug, too].
> + dev_dbg(&dev->dev, "addr 0x%lx regsize %ld spacing %ld irq %x\n",
> + info->io.addr_data, info->io.regsize, info->io.regspacing,
> + info->irq);
Please all addresses/sizes/spacings in hexadecimal? And
you might want to output regshift, too.
> +static int ipmi_of_remove(struct of_device *dev)
> +{
> + /* should call
> + * cleanup_one_si(dev->dev.driver_data); */
So fix that :-)
> + return 0;
> +}
> +static struct of_device_id ipmi_match[] =
> +{
> + { .type = "ipmi", .compatible = "ipmi-kcs", .data = (void*)
> SI_KCS },
> + { .type = "ipmi", .compatible = "ipmi-smic", .data = (void*)
> SI_SMIC },
> + { .type = "ipmi", .compatible = "ipmi-bt", .data = (void*)SI_BT },
That cast-to-pointer is what gives you that warning when
casting back. Is there no better solution?
All pretty minor, but please fix. Looks like you're almost
there :-)
Segher
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
