Re: [PATCHv2 2/3] misc: add support for bmp18x chips to the bmp085 driver

[Arnd Bergmann]

On Tuesday 06 March 2012, Eric Andersson wrote:
> The bmp18x chip family comes in an I2C respectively SPI variant.
> Hence, the bmp085 driver was split to support both buses.
> 
> Tested-by: Zhengguang Guo <zhengguang.guo@xxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Stefan Nilsson <stefan.nilsson@xxxxxxxxxxxxx>
> Signed-off-by: Eric Andersson <eric.andersson@xxxxxxxxxxxxx>

(adding Mark Brown to Cc)

I don't know much about regmap, but your description sounds like you
should be using it here, to simplify the differences between i2c and
spi in your driver instead of implementing your own abstraction
layer.

>  config BMP085
>  	tristate "BMP085 digital pressure sensor"
> -	depends on I2C && SYSFS
> +	depends on (I2C || SPI_MASTER) && SYSFS
>  	help
> -	  If you say yes here you get support for the Bosch Sensortec
> -	  BMP085 digital pressure sensor.
> +	  Say Y here if you want support for Bosch Sensortec's digital
> +	  pressure sensors BMP085 and BMP18x.
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called bmp085.
>  
> +config BMP085_I2C
> +	tristate "support I2C bus connection"
> +	depends on BMP085 && I2C
> +	help
> +	  Say Y here if you want to support Bosch Sensortec's digital pressure
> +	  sensor hooked to an I2C bus.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called bmp085-i2c.
> +
> +config BMP085_SPI
> +	tristate "support SPI bus connection"
> +	depends on BMP085 && SPI_MASTER
> +	help
> +	  Say Y here if you want to support Bosch Sensortec's digital pressure
> +	  sensor hooked to an SPI bus.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called bmp085-spi.
> +

IMHO this would be better expressed if you make CONFIG_BMP085 a hidden
option that is selected when either BMP085_I2C or BMP085_SPI are enabled.
Otherwise you make it possible to build just the base driver but neither
of the front-ends, which is a bit pointless.

>  }
> +EXPORT_SYMBOL(bmp085_probe);
>  

When you add internal symbols inside of your driver, best always use
EXPORT_SYMBOL_GPL. There is little practical difference here, but it
helps to get used to just always pick that as a default.

	Arnd
--
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/