[PATCH v1 00/10] misc: eeprom_93xx46: Get rid of legacy GPIO APIs

[PATCH v1 00/10] misc: eeprom_93xx46: Get rid of legacy GPIO APIs

[Andy Shevchenko]

The driver and its solely consumer (via platform data) are using old
GPIO APIs, convert them for good. On top some spring cleanups and fixes.

Andy Shevchenko (10):
  misc: eeprom_93xx46: Make use of device properties
  eeprom: digsy_mtc: Fix 93xx46 driver probe failure
  eeprom: digsy_mtc: Convert to use GPIO descriptors
  misc: eeprom_93xx46: Hide legacy platform data in the driver
  misc: eeprom_93xx46: Remove ->prepare() and ->finish() customisation
  misc: eeprom_93xx46: Use spi_message_init_with_transfers()
  misc: eeprom_93xx46: Convert to use kstrtox()
  misc: eeprom_93xx46: Replace explicit castings with proper specifiers
  misc: eeprom_93xx46: Use string_choices API instead of ternary
    operator
  misc: eeprom_93xx46: Convert to DEVICE_ATTR_WO()

 drivers/misc/eeprom/digsy_mtc_eeprom.c |  46 +++----
 drivers/misc/eeprom/eeprom_93xx46.c    | 178 ++++++++++++-------------
 include/linux/eeprom_93xx46.h          |  32 -----
 3 files changed, 102 insertions(+), 154 deletions(-)
 delete mode 100644 include/linux/eeprom_93xx46.h

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 01/10] misc: eeprom_93xx46: Make use of device properties

[Andy Shevchenko]

Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.

Include mod_devicetable.h explicitly to replace the dropped of.h
which included mod_devicetable.h indirectly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 41 ++++++++++++++---------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index 45c8ae0db8f9..bbcc9412bb4e 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -10,13 +10,15 @@
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/log2.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+
 #include <linux/nvmem-provider.h>
+
 #include <linux/eeprom_93xx46.h>
 
 #define OP_START	0x4
@@ -422,22 +424,20 @@ static const struct spi_device_id eeprom_93xx46_spi_ids[] = {
 };
 MODULE_DEVICE_TABLE(spi, eeprom_93xx46_spi_ids);
 
-static int eeprom_93xx46_probe_dt(struct spi_device *spi)
+static int eeprom_93xx46_probe_fw(struct device *dev)
 {
-	const struct of_device_id *of_id =
-		of_match_device(eeprom_93xx46_of_table, &spi->dev);
-	struct device_node *np = spi->dev.of_node;
+	const struct eeprom_93xx46_devtype_data *data;
 	struct eeprom_93xx46_platform_data *pd;
 	u32 tmp;
 	int ret;
 
-	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
 	if (!pd)
 		return -ENOMEM;
 
-	ret = of_property_read_u32(np, "data-size", &tmp);
+	ret = device_property_read_u32(dev, "data-size", &tmp);
 	if (ret < 0) {
-		dev_err(&spi->dev, "data-size property not found\n");
+		dev_err(dev, "data-size property not found\n");
 		return ret;
 	}
 
@@ -446,30 +446,28 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
 	} else if (tmp == 16) {
 		pd->flags |= EE_ADDR16;
 	} else {
-		dev_err(&spi->dev, "invalid data-size (%d)\n", tmp);
+		dev_err(dev, "invalid data-size (%d)\n", tmp);
 		return -EINVAL;
 	}
 
-	if (of_property_read_bool(np, "read-only"))
+	if (device_property_read_bool(dev, "read-only"))
 		pd->flags |= EE_READONLY;
 
-	pd->select = devm_gpiod_get_optional(&spi->dev, "select",
-					     GPIOD_OUT_LOW);
+	pd->select = devm_gpiod_get_optional(dev, "select", GPIOD_OUT_LOW);
 	if (IS_ERR(pd->select))
 		return PTR_ERR(pd->select);
+	gpiod_set_consumer_name(pd->select, "93xx46 EEPROMs OE");
 
 	pd->prepare = select_assert;
 	pd->finish = select_deassert;
-	gpiod_direction_output(pd->select, 0);
-
-	if (of_id->data) {
-		const struct eeprom_93xx46_devtype_data *data = of_id->data;
 
+	data = spi_get_device_match_data(to_spi_device(dev));
+	if (data) {
 		pd->quirks = data->quirks;
 		pd->flags |= data->flags;
 	}
 
-	spi->dev.platform_data = pd;
+	dev->platform_data = pd;
 
 	return 0;
 }
@@ -478,10 +476,11 @@ static int eeprom_93xx46_probe(struct spi_device *spi)
 {
 	struct eeprom_93xx46_platform_data *pd;
 	struct eeprom_93xx46_dev *edev;
+	struct device *dev = &spi->dev;
 	int err;
 
-	if (spi->dev.of_node) {
-		err = eeprom_93xx46_probe_dt(spi);
+	if (dev_fwnode(dev)) {
+		err = eeprom_93xx46_probe_fw(dev);
 		if (err < 0)
 			return err;
 	}
@@ -565,7 +564,7 @@ static void eeprom_93xx46_remove(struct spi_device *spi)
 static struct spi_driver eeprom_93xx46_driver = {
 	.driver = {
 		.name	= "93xx46",
-		.of_match_table = of_match_ptr(eeprom_93xx46_of_table),
+		.of_match_table = eeprom_93xx46_of_table,
 	},
 	.probe		= eeprom_93xx46_probe,
 	.remove		= eeprom_93xx46_remove,
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 01/10] misc: eeprom_93xx46: Make use of device properties

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Convert the module to be property provider agnostic and allow
> it to be used on non-OF platforms.
>
> Include mod_devicetable.h explicitly to replace the dropped of.h
> which included mod_devicetable.h indirectly.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 02/10] eeprom: digsy_mtc: Fix 93xx46 driver probe failure

[Andy Shevchenko]

The update to support other (bigger) types of EEPROMs broke
the driver loading due to removal of the default size.

Fix this by adding the respective (new) flag to the platform data.

Fixes: 14374fbb3f06 ("misc: eeprom_93xx46: Add new 93c56 and 93c66 compatible strings")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/digsy_mtc_eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/digsy_mtc_eeprom.c b/drivers/misc/eeprom/digsy_mtc_eeprom.c
index f1f766b70965..4eddc5ba1af9 100644
--- a/drivers/misc/eeprom/digsy_mtc_eeprom.c
+++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c
@@ -42,7 +42,7 @@ static void digsy_mtc_op_finish(void *p)
 }
 
 struct eeprom_93xx46_platform_data digsy_mtc_eeprom_data = {
-	.flags		= EE_ADDR8,
+	.flags		= EE_ADDR8 | EE_SIZE1K,
 	.prepare	= digsy_mtc_op_prepare,
 	.finish		= digsy_mtc_op_finish,
 };
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 03/10] eeprom: digsy_mtc: Convert to use GPIO descriptors

[Andy Shevchenko]

This converts the driver to use GPIO descriptors exclusively
to retrieve GPIO lines. Drop the old GPIO handling in favor of
the core managing it exclusively.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/digsy_mtc_eeprom.c | 46 +++++++++++---------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/misc/eeprom/digsy_mtc_eeprom.c b/drivers/misc/eeprom/digsy_mtc_eeprom.c
index 4eddc5ba1af9..88888485e6f8 100644
--- a/drivers/misc/eeprom/digsy_mtc_eeprom.c
+++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c
@@ -14,13 +14,12 @@
  * and delete this driver.
  */
 
-#include <linux/gpio.h>
 #include <linux/gpio/machine.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_gpio.h>
-#include <linux/eeprom_93xx46.h>
 
 #define GPIO_EEPROM_CLK		216
 #define GPIO_EEPROM_CS		210
@@ -29,22 +28,13 @@
 #define GPIO_EEPROM_OE		255
 #define EE_SPI_BUS_NUM	1
 
-static void digsy_mtc_op_prepare(void *p)
-{
-	/* enable */
-	gpio_set_value(GPIO_EEPROM_OE, 0);
-}
+static const struct property_entry digsy_mtc_spi_properties[] = {
+	PROPERTY_ENTRY_U32("data-size", 8),
+	{ }
+};
 
-static void digsy_mtc_op_finish(void *p)
-{
-	/* disable */
-	gpio_set_value(GPIO_EEPROM_OE, 1);
-}
-
-struct eeprom_93xx46_platform_data digsy_mtc_eeprom_data = {
-	.flags		= EE_ADDR8 | EE_SIZE1K,
-	.prepare	= digsy_mtc_op_prepare,
-	.finish		= digsy_mtc_op_finish,
+static const struct software_node digsy_mtc_spi_node = {
+	.properties = digsy_mtc_spi_properties,
 };
 
 static struct spi_gpio_platform_data eeprom_spi_gpio_data = {
@@ -70,18 +60,19 @@ static struct gpiod_lookup_table eeprom_spi_gpiod_table = {
 			    "miso", GPIO_ACTIVE_HIGH),
 		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_CS,
 			    "cs", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio@b00", GPIO_EEPROM_OE,
+			    "select", GPIO_ACTIVE_LOW),
 		{ },
 	},
 };
 
 static struct spi_board_info digsy_mtc_eeprom_info[] __initdata = {
 	{
-		.modalias		= "93xx46",
+		.modalias		= "eeprom-93xx46",
 		.max_speed_hz		= 1000000,
 		.bus_num		= EE_SPI_BUS_NUM,
 		.chip_select		= 0,
 		.mode			= SPI_MODE_0,
-		.platform_data		= &digsy_mtc_eeprom_data,
 	},
 };
 
@@ -89,15 +80,18 @@ static int __init digsy_mtc_eeprom_devices_init(void)
 {
 	int ret;
 
-	ret = gpio_request_one(GPIO_EEPROM_OE, GPIOF_OUT_INIT_HIGH,
-				"93xx46 EEPROMs OE");
-	if (ret) {
-		pr_err("can't request gpio %d\n", GPIO_EEPROM_OE);
-		return ret;
-	}
 	gpiod_add_lookup_table(&eeprom_spi_gpiod_table);
 	spi_register_board_info(digsy_mtc_eeprom_info,
 				ARRAY_SIZE(digsy_mtc_eeprom_info));
-	return platform_device_register(&digsy_mtc_eeprom);
+
+	ret = device_add_software_node(&digsy_mtc_eeprom.dev, &digsy_mtc_spi_node);
+	if (ret)
+		return ret;
+
+	ret = platform_device_register(&digsy_mtc_eeprom);
+	if (ret)
+		device_remove_software_node(&digsy_mtc_eeprom.dev);
+
+	return ret;
 }
 device_initcall(digsy_mtc_eeprom_devices_init);
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 03/10] eeprom: digsy_mtc: Convert to use GPIO descriptors

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> This converts the driver to use GPIO descriptors exclusively
> to retrieve GPIO lines. Drop the old GPIO handling in favor of
> the core managing it exclusively.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 04/10] misc: eeprom_93xx46: Hide legacy platform data in the driver

[Andy Shevchenko]

First of all, there is no user for the platform data in the kernel.
Second, it needs a lot of updates to follow the modern standards
of the kernel, including proper Device Tree bindings and device
property handling.

For now, just hide the legacy platform data in the driver's code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 35 ++++++++++++++++++++++++-----
 include/linux/eeprom_93xx46.h       | 32 --------------------------
 2 files changed, 29 insertions(+), 38 deletions(-)
 delete mode 100644 include/linux/eeprom_93xx46.h

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index bbcc9412bb4e..a5a043ddedbb 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -5,6 +5,7 @@
  * (C) 2011 DENX Software Engineering, Anatolij Gustschin <agust@denx.de>
  */
 
+#include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
@@ -19,7 +20,31 @@
 
 #include <linux/nvmem-provider.h>
 
-#include <linux/eeprom_93xx46.h>
+struct eeprom_93xx46_platform_data {
+	unsigned char	flags;
+#define EE_ADDR8	0x01		/*  8 bit addr. cfg */
+#define EE_ADDR16	0x02		/* 16 bit addr. cfg */
+#define EE_READONLY	0x08		/* forbid writing */
+#define EE_SIZE1K	0x10		/* 1 kb of data, that is a 93xx46 */
+#define EE_SIZE2K	0x20		/* 2 kb of data, that is a 93xx56 */
+#define EE_SIZE4K	0x40		/* 4 kb of data, that is a 93xx66 */
+
+	unsigned int	quirks;
+/* Single word read transfers only; no sequential read. */
+#define EEPROM_93XX46_QUIRK_SINGLE_WORD_READ		(1 << 0)
+/* Instructions such as EWEN are (addrlen + 2) in length. */
+#define EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH		(1 << 1)
+/* Add extra cycle after address during a read */
+#define EEPROM_93XX46_QUIRK_EXTRA_READ_CYCLE		BIT(2)
+
+	/*
+	 * optional hooks to control additional logic
+	 * before and after spi transfer.
+	 */
+	void (*prepare)(void *);
+	void (*finish)(void *);
+	struct gpio_desc *select;
+};
 
 #define OP_START	0x4
 #define OP_WRITE	(OP_START | 0x1)
@@ -479,11 +504,9 @@ static int eeprom_93xx46_probe(struct spi_device *spi)
 	struct device *dev = &spi->dev;
 	int err;
 
-	if (dev_fwnode(dev)) {
-		err = eeprom_93xx46_probe_fw(dev);
-		if (err < 0)
-			return err;
-	}
+	err = eeprom_93xx46_probe_fw(dev);
+	if (err < 0)
+		return err;
 
 	pd = spi->dev.platform_data;
 	if (!pd) {
diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
deleted file mode 100644
index 34c2175e6a1e..000000000000
--- a/include/linux/eeprom_93xx46.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Module: eeprom_93xx46
- * platform description for 93xx46 EEPROMs.
- */
-#include <linux/gpio/consumer.h>
-
-struct eeprom_93xx46_platform_data {
-	unsigned char	flags;
-#define EE_ADDR8	0x01		/*  8 bit addr. cfg */
-#define EE_ADDR16	0x02		/* 16 bit addr. cfg */
-#define EE_READONLY	0x08		/* forbid writing */
-#define EE_SIZE1K	0x10		/* 1 kb of data, that is a 93xx46 */
-#define EE_SIZE2K	0x20		/* 2 kb of data, that is a 93xx56 */
-#define EE_SIZE4K	0x40		/* 4 kb of data, that is a 93xx66 */
-
-	unsigned int	quirks;
-/* Single word read transfers only; no sequential read. */
-#define EEPROM_93XX46_QUIRK_SINGLE_WORD_READ		(1 << 0)
-/* Instructions such as EWEN are (addrlen + 2) in length. */
-#define EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH		(1 << 1)
-/* Add extra cycle after address during a read */
-#define EEPROM_93XX46_QUIRK_EXTRA_READ_CYCLE		BIT(2)
-
-	/*
-	 * optional hooks to control additional logic
-	 * before and after spi transfer.
-	 */
-	void (*prepare)(void *);
-	void (*finish)(void *);
-	struct gpio_desc *select;
-};
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 04/10] misc: eeprom_93xx46: Hide legacy platform data in the driver

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> First of all, there is no user for the platform data in the kernel.
> Second, it needs a lot of updates to follow the modern standards
> of the kernel, including proper Device Tree bindings and device
> property handling.
>
> For now, just hide the legacy platform data in the driver's code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 05/10] misc: eeprom_93xx46: Remove ->prepare() and ->finish() customisation

[Andy Shevchenko]

Currently there is only one way how chip is prepared and unprepared
for an operation. Drop unnecessary customisation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 48 +++++------------------------
 1 file changed, 8 insertions(+), 40 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index a5a043ddedbb..b6d699c1cd39 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -37,12 +37,6 @@ struct eeprom_93xx46_platform_data {
 /* Add extra cycle after address during a read */
 #define EEPROM_93XX46_QUIRK_EXTRA_READ_CYCLE		BIT(2)
 
-	/*
-	 * optional hooks to control additional logic
-	 * before and after spi transfer.
-	 */
-	void (*prepare)(void *);
-	void (*finish)(void *);
 	struct gpio_desc *select;
 };
 
@@ -123,8 +117,7 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
 
 	mutex_lock(&edev->lock);
 
-	if (edev->pdata->prepare)
-		edev->pdata->prepare(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 1);
 
 	/* The opcode in front of the address is three bits. */
 	bits = edev->addrlen + 3;
@@ -180,8 +173,7 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
 		count -= nbytes;
 	}
 
-	if (edev->pdata->finish)
-		edev->pdata->finish(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 0);
 
 	mutex_unlock(&edev->lock);
 
@@ -222,8 +214,7 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 
 	mutex_lock(&edev->lock);
 
-	if (edev->pdata->prepare)
-		edev->pdata->prepare(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 1);
 
 	ret = spi_sync(edev->spi, &m);
 	/* have to wait at least Tcsl ns */
@@ -232,8 +223,7 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 		dev_err(&edev->spi->dev, "erase/write %sable error %d\n",
 			is_on ? "en" : "dis", ret);
 
-	if (edev->pdata->finish)
-		edev->pdata->finish(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 0);
 
 	mutex_unlock(&edev->lock);
 	return ret;
@@ -312,8 +302,7 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
 
 	mutex_lock(&edev->lock);
 
-	if (edev->pdata->prepare)
-		edev->pdata->prepare(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 1);
 
 	for (i = 0; i < count; i += step) {
 		ret = eeprom_93xx46_write_word(edev, &buf[i], off + i);
@@ -324,8 +313,7 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
 		}
 	}
 
-	if (edev->pdata->finish)
-		edev->pdata->finish(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 0);
 
 	mutex_unlock(&edev->lock);
 
@@ -336,7 +324,6 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
 
 static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 {
-	struct eeprom_93xx46_platform_data *pd = edev->pdata;
 	struct spi_message m;
 	struct spi_transfer t;
 	int bits, ret;
@@ -368,8 +355,7 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 
 	mutex_lock(&edev->lock);
 
-	if (edev->pdata->prepare)
-		edev->pdata->prepare(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 1);
 
 	ret = spi_sync(edev->spi, &m);
 	if (ret)
@@ -377,8 +363,7 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 	/* have to wait erase cycle time Tec ms */
 	mdelay(6);
 
-	if (pd->finish)
-		pd->finish(edev);
+	gpiod_set_value_cansleep(edev->pdata->select, 0);
 
 	mutex_unlock(&edev->lock);
 	return ret;
@@ -407,20 +392,6 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
 }
 static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
 
-static void select_assert(void *context)
-{
-	struct eeprom_93xx46_dev *edev = context;
-
-	gpiod_set_value_cansleep(edev->pdata->select, 1);
-}
-
-static void select_deassert(void *context)
-{
-	struct eeprom_93xx46_dev *edev = context;
-
-	gpiod_set_value_cansleep(edev->pdata->select, 0);
-}
-
 static const struct of_device_id eeprom_93xx46_of_table[] = {
 	{ .compatible = "eeprom-93xx46", .data = &at93c46_data, },
 	{ .compatible = "atmel,at93c46", .data = &at93c46_data, },
@@ -483,9 +454,6 @@ static int eeprom_93xx46_probe_fw(struct device *dev)
 		return PTR_ERR(pd->select);
 	gpiod_set_consumer_name(pd->select, "93xx46 EEPROMs OE");
 
-	pd->prepare = select_assert;
-	pd->finish = select_deassert;
-
 	data = spi_get_device_match_data(to_spi_device(dev));
 	if (data) {
 		pd->quirks = data->quirks;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 05/10] misc: eeprom_93xx46: Remove ->prepare() and ->finish() customisation

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Currently there is only one way how chip is prepared and unprepared
> for an operation. Drop unnecessary customisation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Classic upfront design. Good patch!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 06/10] misc: eeprom_93xx46: Use spi_message_init_with_transfers()

[Andy Shevchenko]

Replace open coded spi_message_init_with_transfers().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 34 +++++++++++------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index b6d699c1cd39..3f885bac72c2 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -5,6 +5,7 @@
  * (C) 2011 DENX Software Engineering, Anatolij Gustschin <agust@denx.de>
  */
 
+#include <linux/array_size.h>
 #include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -124,7 +125,7 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
 
 	while (count) {
 		struct spi_message m;
-		struct spi_transfer t[2] = { { 0 } };
+		struct spi_transfer t[2] = {};
 		u16 cmd_addr = OP_READ << edev->addrlen;
 		size_t nbytes = count;
 
@@ -146,17 +147,15 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
 			bits += 1;
 		}
 
-		spi_message_init(&m);
-
 		t[0].tx_buf = (char *)&cmd_addr;
 		t[0].len = 2;
 		t[0].bits_per_word = bits;
-		spi_message_add_tail(&t[0], &m);
 
 		t[1].rx_buf = buf;
 		t[1].len = count;
 		t[1].bits_per_word = 8;
-		spi_message_add_tail(&t[1], &m);
+
+		spi_message_init_with_transfers(&m, t, ARRAY_SIZE(t));
 
 		err = spi_sync(edev->spi, &m);
 		/* have to wait at least Tcsl ns */
@@ -183,7 +182,7 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
 static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 {
 	struct spi_message m;
-	struct spi_transfer t;
+	struct spi_transfer t = {};
 	int bits, ret;
 	u16 cmd_addr;
 
@@ -204,13 +203,11 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 	dev_dbg(&edev->spi->dev, "ew%s cmd 0x%04x, %d bits\n",
 			is_on ? "en" : "ds", cmd_addr, bits);
 
-	spi_message_init(&m);
-	memset(&t, 0, sizeof(t));
-
 	t.tx_buf = &cmd_addr;
 	t.len = 2;
 	t.bits_per_word = bits;
-	spi_message_add_tail(&t, &m);
+
+	spi_message_init_with_transfers(&m, &t, 1);
 
 	mutex_lock(&edev->lock);
 
@@ -234,7 +231,7 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
 			 const char *buf, unsigned off)
 {
 	struct spi_message m;
-	struct spi_transfer t[2];
+	struct spi_transfer t[2] = {};
 	int bits, data_len, ret;
 	u16 cmd_addr;
 
@@ -256,18 +253,15 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
 
 	dev_dbg(&edev->spi->dev, "write cmd 0x%x\n", cmd_addr);
 
-	spi_message_init(&m);
-	memset(t, 0, sizeof(t));
-
 	t[0].tx_buf = (char *)&cmd_addr;
 	t[0].len = 2;
 	t[0].bits_per_word = bits;
-	spi_message_add_tail(&t[0], &m);
 
 	t[1].tx_buf = buf;
 	t[1].len = data_len;
 	t[1].bits_per_word = 8;
-	spi_message_add_tail(&t[1], &m);
+
+	spi_message_init_with_transfers(&m, t, ARRAY_SIZE(t));
 
 	ret = spi_sync(edev->spi, &m);
 	/* have to wait program cycle time Twc ms */
@@ -325,7 +319,7 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
 static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 {
 	struct spi_message m;
-	struct spi_transfer t;
+	struct spi_transfer t = {};
 	int bits, ret;
 	u16 cmd_addr;
 
@@ -345,13 +339,11 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 
 	dev_dbg(&edev->spi->dev, "eral cmd 0x%04x, %d bits\n", cmd_addr, bits);
 
-	spi_message_init(&m);
-	memset(&t, 0, sizeof(t));
-
 	t.tx_buf = &cmd_addr;
 	t.len = 2;
 	t.bits_per_word = bits;
-	spi_message_add_tail(&t, &m);
+
+	spi_message_init_with_transfers(&m, &t, 1);
 
 	mutex_lock(&edev->lock);
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 07/10] misc: eeprom_93xx46: Convert to use kstrtox()

[Andy Shevchenko]

sscanf() is a heavy one and moreover requires additional boundary checks.
Convert driver to use kstrtobool() in eeprom_93xx46_store_erase().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index 3f885bac72c2..18a3b534ea73 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -10,7 +10,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
-#include <linux/kernel.h>
+#include <linux/kstrtox.h>
 #include <linux/log2.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -366,9 +366,13 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
 					 const char *buf, size_t count)
 {
 	struct eeprom_93xx46_dev *edev = dev_get_drvdata(dev);
-	int erase = 0, ret;
+	bool erase;
+	int ret;
+
+	ret = kstrtobool(buf, &erase);
+	if (ret)
+		return ret;
 
-	sscanf(buf, "%d", &erase);
 	if (erase) {
 		ret = eeprom_93xx46_ew(edev, 1);
 		if (ret)
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 07/10] misc: eeprom_93xx46: Convert to use kstrtox()

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> sscanf() is a heavy one and moreover requires additional boundary checks.
> Convert driver to use kstrtobool() in eeprom_93xx46_store_erase().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 08/10] misc: eeprom_93xx46: Replace explicit castings with proper specifiers

[Andy Shevchenko]

There is no need to have an explicit casting when we can simply use
the correct printf() specifier.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index 18a3b534ea73..ac485b2827db 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -162,8 +162,8 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
 		ndelay(250);
 
 		if (err) {
-			dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
-				nbytes, (int)off, err);
+			dev_err(&edev->spi->dev, "read %zu bytes at %u: err. %d\n",
+				nbytes, off, err);
 			break;
 		}
 
@@ -274,7 +274,8 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
 {
 	struct eeprom_93xx46_dev *edev = priv;
 	char *buf = val;
-	int i, ret, step = 1;
+	int ret, step = 1;
+	unsigned int i;
 
 	if (unlikely(off >= edev->size))
 		return -EFBIG;
@@ -301,8 +302,7 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
 	for (i = 0; i < count; i += step) {
 		ret = eeprom_93xx46_write_word(edev, &buf[i], off + i);
 		if (ret) {
-			dev_err(&edev->spi->dev, "write failed at %d: %d\n",
-				(int)off + i, ret);
+			dev_err(&edev->spi->dev, "write failed at %u: %d\n", off + i, ret);
 			break;
 		}
 	}
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 08/10] misc: eeprom_93xx46: Replace explicit castings with proper specifiers

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> There is no need to have an explicit casting when we can simply use
> the correct printf() specifier.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 09/10] misc: eeprom_93xx46: Use string_choices API instead of ternary operator

[Andy Shevchenko]

Use modern string_choices API instead of manually determining the
output using ternary operator.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index ac485b2827db..ad3b3bc054da 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -18,6 +18,7 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+#include <linux/string_choices.h>
 
 #include <linux/nvmem-provider.h>
 
@@ -200,8 +201,8 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 		bits += 2;
 	}
 
-	dev_dbg(&edev->spi->dev, "ew%s cmd 0x%04x, %d bits\n",
-			is_on ? "en" : "ds", cmd_addr, bits);
+	dev_dbg(&edev->spi->dev, "ew %s cmd 0x%04x, %d bits\n",
+		str_enable_disable(is_on), cmd_addr, bits);
 
 	t.tx_buf = &cmd_addr;
 	t.len = 2;
@@ -217,8 +218,8 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 	/* have to wait at least Tcsl ns */
 	ndelay(250);
 	if (ret)
-		dev_err(&edev->spi->dev, "erase/write %sable error %d\n",
-			is_on ? "en" : "dis", ret);
+		dev_err(&edev->spi->dev, "erase/write %s error %d\n",
+			str_enable_disable(is_on), ret);
 
 	gpiod_set_value_cansleep(edev->pdata->select, 0);
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 09/10] misc: eeprom_93xx46: Use string_choices API instead of ternary operator

[Linus Walleij]

On Wed, May 8, 2024 at 8:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Use modern string_choices API instead of manually determining the
> output using ternary operator.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Hm. Neat actually. Starting to like the string_chioces.h more.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 10/10] misc: eeprom_93xx46: Convert to DEVICE_ATTR_WO()

[Andy Shevchenko]

Use DEVICE_ATTR_WO() helper instead of plain DEVICE_ATTR(),
which makes the code a bit shorter and easier to read.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index ad3b3bc054da..e2221be88445 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -362,9 +362,8 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 	return ret;
 }
 
-static ssize_t eeprom_93xx46_store_erase(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t count)
+static ssize_t erase_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
 {
 	struct eeprom_93xx46_dev *edev = dev_get_drvdata(dev);
 	bool erase;
@@ -387,7 +386,7 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
 	}
 	return count;
 }
-static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
+static DEVICE_ATTR_WO(erase);
 
 static const struct of_device_id eeprom_93xx46_of_table[] = {
 	{ .compatible = "eeprom-93xx46", .data = &at93c46_data, },
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 10/10] misc: eeprom_93xx46: Convert to DEVICE_ATTR_WO()

[Linus Walleij]

On Wed, May 8, 2024 at 8:50 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Use DEVICE_ATTR_WO() helper instead of plain DEVICE_ATTR(),
> which makes the code a bit shorter and easier to read.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij