dmitry pervushin <[email protected]> wrote:
>
A few coding style nits.
> +#include "spi_locals.h"
> +
> +static LIST_HEAD( spi_busses );
Please don't put spaces after '(' and before ')'.
> + if (0 == strncmp(*id, SPI_ID_ANY, strlen(SPI_ID_ANY))) {
And this trick isn't really needed. If you do
if (whatever = constant)
then the compiler will generate a warning. Please do these comparisons in
the conventional way, with the constant on the right hand side.
> + if( bus ) {
> + init_MUTEX( &bus->lock );
> +
> + bus->platform_device.name = NULL;
> + bus->the_bus.name = NULL;
> +
> + strncpy( busname, name ? name : "SPI", sizeof( busname ) );
Lots more extraneous spaces after '(' and before ')'.
> + if( bus->the_bus.name ) {
> + strcpy( bus->the_bus.name, fullname );
> + }
No braces here.
> + err = bus_register( &bus->the_bus );
> + if( err ) {
> + goto out;
> + }
And here.
> + list_add_tail( &bus->bus_list, &spi_busses );
> + bus->platform_device.name = kmalloc( strlen( busname )+1, GFP_KERNEL );
> + if( bus->platform_device.name ) {
> + strcpy( bus->platform_device.name, busname );
> + }
and here...
> +void spi_bus_unregister( struct spi_bus* bus )
> +{
> + if( bus ) {
We do put a space after `if', so this line should be
if (bus) {
> +struct spi_bus* spi_bus_find( char* id )
The asterisk goes with the variable, not with the type. So the above should be
struct spi_bus *spi_bus_find(char *id)
> +int spi_device_add( struct spi_bus* bus, struct spi_device *dev, char* name)
Here too.
> +int spi_do_probe( struct device* dev, void* device_driver )
You seem to have an awful lot of non-static functions. Please check
whether they all really need to have global scope.
> + if (NULL == dev) {
if (dev == NULL) {
> +static int spidev_do_open(struct device *the_dev, void *context)
> +{
> + struct spidev_openclose *o = (struct spidev_openclose *) context;
Don't typecast void* when assigning to and from pointers. It adds clutter
and defeats typechecking.
> + struct spi_device *dev = SPI_DEV(the_dev);
> + struct spidev_driver_data *drvdata;
> +
> + drvdata = (struct spidev_driver_data *) dev_get_drvdata(the_dev);
Ditto.
> +
> + out_unreg:
Labels go in column zero.
> + void *(*alloc) (size_t, int);
> + void (*free) (const void *);
> + unsigned long (*copy_from_user) (void *to, const void *from_user,
> + unsigned long len);
> + unsigned long (*copy_to_user) (void *to_user, const void *from,
> + unsigned long len);
The above names are risky. Some platform may implement copy_to_user() as a
macro.
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |