Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1459984

Re: [PATCH] eeprom: at24: check if the chip is functional in probe()

From Wolfram Sang <wsa@the-dreams.de>
Newsgroups linux.kernel
Subject Re: [PATCH] eeprom: at24: check if the chip is functional in probe()
Date 2016-08-10 23:30 +0200
Message-ID <s4Jez-29X-25@gated-at.bofh.it> (permalink)
References <s4H36-I0-109@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Wed, Aug 10, 2016 at 03:54:17PM +0200, Bartosz Golaszewski wrote:
> The at24 driver doesn't check if the chip is functional in its probe
> function. This leads to instantiating devices that are not physically
> present. For example the cape EEPROMs for BeagleBone Black are defined
> in the device tree at four addresses on i2c2, but normally only one of
> them is present.
> 
> If the userspace doesn't know the location in advance, it will need to
> check if reading the nvmem attributes fails to determine which EEPROM
> is actually there.
> 
> Try to read a single byte in probe() and bail-out with -ENODEV if the
> read fails.

That's basically OK...

> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/misc/eeprom/at24.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 3cdf8e1..ed1e4eb 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -593,6 +593,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	struct at24_data *at24;
>  	int err;
>  	unsigned i, num_addresses;
> +	char c;

u8?

>  
>  	if (client->dev.platform_data) {
>  		chip = *(struct at24_platform_data *)client->dev.platform_data;
> @@ -780,6 +781,15 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	if (chip.setup)
>  		chip.setup(at24->nvmem, chip.context);
>  
> +	err = at24_read(at24, 0, &c, 1);

Can't we do this before registering dummy clients and nvmem registration?

> +	if (err) {
> +		dev_err(&client->dev,
> +			"error reading the test byte from EEPROM: %d\n", err);

I don't think we should print an error in case of ENODEV.

> +		nvmem_unregister(at24->nvmem);
> +		err = -ENODEV;
> +		goto err_clients;
> +	}
> +
>  	return 0;
>  
>  err_clients:
> -- 
> 2.7.4
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] eeprom: at24: check if the chip is functional in probe() Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-08-10 21:10 +0200
  Re: [PATCH] eeprom: at24: check if the chip is functional in probe() Wolfram Sang <wsa@the-dreams.de> - 2016-08-10 23:30 +0200
    Re: [PATCH] eeprom: at24: check if the chip is functional in probe() Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-08-11 17:20 +0200
      Re: [PATCH] eeprom: at24: check if the chip is functional in probe() Wolfram Sang <wsa@the-dreams.de> - 2016-08-11 18:00 +0200

csiph-web