linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] misc: eeprom: at24: Always append device id even if label property is set.
@ 2021-07-01 15:28 jglisse
  2021-07-01 16:47 ` Bartosz Golaszewski
  0 siblings, 1 reply; 3+ messages in thread
From: jglisse @ 2021-07-01 15:28 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jérôme Glisse, Diego Santa Cruz, Bartosz Golaszewski,
	Jon Hunter, stable, linux-i2c

From: Jérôme Glisse <jglisse@redhat.com>

We need to append device id even if eeprom have a label property set as some
platform can have multiple eeproms with same label and we can not register
each of those with same label. Failing to register those eeproms trigger
cascade failures on such platform (system is no longer working).

This fix regression on such platform introduced with 4e302c3b568e

Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
Cc: Diego Santa Cruz <Diego.SantaCruz@spinetix.com>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: stable@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
---
 drivers/misc/eeprom/at24.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 7a6f01ace78a..305ffad131a2 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -714,23 +714,20 @@ static int at24_probe(struct i2c_client *client)
 	}
 
 	/*
-	 * If the 'label' property is not present for the AT24 EEPROM,
-	 * then nvmem_config.id is initialised to NVMEM_DEVID_AUTO,
-	 * and this will append the 'devid' to the name of the NVMEM
-	 * device. This is purely legacy and the AT24 driver has always
-	 * defaulted to this. However, if the 'label' property is
-	 * present then this means that the name is specified by the
-	 * firmware and this name should be used verbatim and so it is
-	 * not necessary to append the 'devid'.
+	 * We initialize nvmem_config.id to NVMEM_DEVID_AUTO even if the
+	 * label property is set as some platform can have multiple eeproms
+	 * with same label and we can not register each of those with same
+	 * label. Failing to register those eeproms trigger cascade failure
+	 * on such platform.
 	 */
+	nvmem_config.id = NVMEM_DEVID_AUTO;
+
 	if (device_property_present(dev, "label")) {
-		nvmem_config.id = NVMEM_DEVID_NONE;
 		err = device_property_read_string(dev, "label",
 						  &nvmem_config.name);
 		if (err)
 			return err;
 	} else {
-		nvmem_config.id = NVMEM_DEVID_AUTO;
 		nvmem_config.name = dev_name(dev);
 	}
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] misc: eeprom: at24: Always append device id even if label property is set.
  2021-07-01 15:28 [PATCH] misc: eeprom: at24: Always append device id even if label property is set jglisse
@ 2021-07-01 16:47 ` Bartosz Golaszewski
  2021-07-05 16:50   ` Alexander Fomichev
  0 siblings, 1 reply; 3+ messages in thread
From: Bartosz Golaszewski @ 2021-07-01 16:47 UTC (permalink / raw)
  To: Jérôme Glisse
  Cc: LKML, Diego Santa Cruz, Jon Hunter, Stable # 4 . 20+,
	linux-i2c, Alexander Fomichev

On Thu, Jul 1, 2021 at 5:28 PM <jglisse@redhat.com> wrote:
>
> From: Jérôme Glisse <jglisse@redhat.com>
>
> We need to append device id even if eeprom have a label property set as some
> platform can have multiple eeproms with same label and we can not register
> each of those with same label. Failing to register those eeproms trigger
> cascade failures on such platform (system is no longer working).
>
> This fix regression on such platform introduced with 4e302c3b568e
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Diego Santa Cruz <Diego.SantaCruz@spinetix.com>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: stable@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> ---
>  drivers/misc/eeprom/at24.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 7a6f01ace78a..305ffad131a2 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -714,23 +714,20 @@ static int at24_probe(struct i2c_client *client)
>         }
>
>         /*
> -        * If the 'label' property is not present for the AT24 EEPROM,
> -        * then nvmem_config.id is initialised to NVMEM_DEVID_AUTO,
> -        * and this will append the 'devid' to the name of the NVMEM
> -        * device. This is purely legacy and the AT24 driver has always
> -        * defaulted to this. However, if the 'label' property is
> -        * present then this means that the name is specified by the
> -        * firmware and this name should be used verbatim and so it is
> -        * not necessary to append the 'devid'.
> +        * We initialize nvmem_config.id to NVMEM_DEVID_AUTO even if the
> +        * label property is set as some platform can have multiple eeproms
> +        * with same label and we can not register each of those with same
> +        * label. Failing to register those eeproms trigger cascade failure
> +        * on such platform.
>          */
> +       nvmem_config.id = NVMEM_DEVID_AUTO;
> +
>         if (device_property_present(dev, "label")) {
> -               nvmem_config.id = NVMEM_DEVID_NONE;
>                 err = device_property_read_string(dev, "label",
>                                                   &nvmem_config.name);
>                 if (err)
>                         return err;
>         } else {
> -               nvmem_config.id = NVMEM_DEVID_AUTO;
>                 nvmem_config.name = dev_name(dev);
>         }
>
> --
> 2.31.1
>

Cc'ing Alexander Fomichev who reported this issue first.

This is the second time someone raises this problem so it seems that
this change really broke many existing systems. I will apply this
patch and send it for stable.

Bart

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] misc: eeprom: at24: Always append device id even if label property is set.
  2021-07-01 16:47 ` Bartosz Golaszewski
@ 2021-07-05 16:50   ` Alexander Fomichev
  0 siblings, 0 replies; 3+ messages in thread
From: Alexander Fomichev @ 2021-07-05 16:50 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Jérôme Glisse, LKML, Diego Santa Cruz, Jon Hunter,
	Stable # 4 . 20+,
	linux-i2c, linux

The proposed patch has been tested and it solves the problem.
Thanks.

Tested-by: Alexander Fomichev <fomichev.ru@gmail.com>

чт, 1 июл. 2021 г. в 19:47, Bartosz Golaszewski <bgolaszewski@baylibre.com>:
>
> On Thu, Jul 1, 2021 at 5:28 PM <jglisse@redhat.com> wrote:
> >
> > From: Jérôme Glisse <jglisse@redhat.com>
> >
> > We need to append device id even if eeprom have a label property set as some
> > platform can have multiple eeproms with same label and we can not register
> > each of those with same label. Failing to register those eeproms trigger
> > cascade failures on such platform (system is no longer working).
> >
> > This fix regression on such platform introduced with 4e302c3b568e
> >
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Cc: Diego Santa Cruz <Diego.SantaCruz@spinetix.com>
> > Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Cc: Jon Hunter <jonathanh@nvidia.com>
> > Cc: stable@vger.kernel.org
> > Cc: linux-i2c@vger.kernel.org
> > ---
> >  drivers/misc/eeprom/at24.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> > index 7a6f01ace78a..305ffad131a2 100644
> > --- a/drivers/misc/eeprom/at24.c
> > +++ b/drivers/misc/eeprom/at24.c
> > @@ -714,23 +714,20 @@ static int at24_probe(struct i2c_client *client)
> >         }
> >
> >         /*
> > -        * If the 'label' property is not present for the AT24 EEPROM,
> > -        * then nvmem_config.id is initialised to NVMEM_DEVID_AUTO,
> > -        * and this will append the 'devid' to the name of the NVMEM
> > -        * device. This is purely legacy and the AT24 driver has always
> > -        * defaulted to this. However, if the 'label' property is
> > -        * present then this means that the name is specified by the
> > -        * firmware and this name should be used verbatim and so it is
> > -        * not necessary to append the 'devid'.
> > +        * We initialize nvmem_config.id to NVMEM_DEVID_AUTO even if the
> > +        * label property is set as some platform can have multiple eeproms
> > +        * with same label and we can not register each of those with same
> > +        * label. Failing to register those eeproms trigger cascade failure
> > +        * on such platform.
> >          */
> > +       nvmem_config.id = NVMEM_DEVID_AUTO;
> > +
> >         if (device_property_present(dev, "label")) {
> > -               nvmem_config.id = NVMEM_DEVID_NONE;
> >                 err = device_property_read_string(dev, "label",
> >                                                   &nvmem_config.name);
> >                 if (err)
> >                         return err;
> >         } else {
> > -               nvmem_config.id = NVMEM_DEVID_AUTO;
> >                 nvmem_config.name = dev_name(dev);
> >         }
> >
> > --
> > 2.31.1
> >
>
> Cc'ing Alexander Fomichev who reported this issue first.
>
> This is the second time someone raises this problem so it seems that
> this change really broke many existing systems. I will apply this
> patch and send it for stable.
>
> Bart



-- 
С уважением,
Александр Фомичёв.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-07-05 16:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 15:28 [PATCH] misc: eeprom: at24: Always append device id even if label property is set jglisse
2021-07-01 16:47 ` Bartosz Golaszewski
2021-07-05 16:50   ` Alexander Fomichev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).