[PATCH 1/2] hwmon: sis5595: Introduce a #define for the driver name and use it

[PATCH 1/2] hwmon: sis5595: Introduce a #define for the driver name and use it

[Uwe Kleine-König]

Make use of the cpp symbol DRIVER_NAME to set the driver's name and use
it instead of all explicit usages of the same string. Also make use of
it instead of sis5595_driver.driver.name which breaks a cyclic dependency
between sis5595_probe() and sis5595_driver that in the next commit allows
to drop some forward declarations.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwmon/sis5595.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/sis5595.c b/drivers/hwmon/sis5595.c
index 018cb5a7651f..013f87da6fff 100644
--- a/drivers/hwmon/sis5595.c
+++ b/drivers/hwmon/sis5595.c
@@ -37,6 +37,7 @@
  *	 735		0008		0735
  */
 
+#define DRIVER_NAME "sis5595"
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/module.h>
@@ -201,7 +202,7 @@ static void sis5595_init_device(struct sis5595_data *data);
 
 static struct platform_driver sis5595_driver = {
 	.driver = {
-		.name	= "sis5595",
+		.name	= DRIVER_NAME,
 	},
 	.probe		= sis5595_probe,
 	.remove		= sis5595_remove,
@@ -580,7 +581,7 @@ static int sis5595_probe(struct platform_device *pdev)
 	/* Reserve the ISA region */
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
 	if (!devm_request_region(&pdev->dev, res->start, SIS5595_EXTENT,
-				 sis5595_driver.driver.name))
+				 DRIVER_NAME))
 		return -EBUSY;
 
 	data = devm_kzalloc(&pdev->dev, sizeof(struct sis5595_data),
@@ -591,7 +592,7 @@ static int sis5595_probe(struct platform_device *pdev)
 	mutex_init(&data->lock);
 	mutex_init(&data->update_lock);
 	data->addr = res->start;
-	data->name = "sis5595";
+	data->name = DRIVER_NAME;
 	platform_set_drvdata(pdev, data);
 
 	/*
@@ -764,7 +765,7 @@ static int sis5595_device_add(unsigned short address)
 	struct resource res = {
 		.start	= address,
 		.end	= address + SIS5595_EXTENT - 1,
-		.name	= "sis5595",
+		.name	= DRIVER_NAME,
 		.flags	= IORESOURCE_IO,
 	};
 	int err;
@@ -773,7 +774,7 @@ static int sis5595_device_add(unsigned short address)
 	if (err)
 		goto exit;
 
-	pdev = platform_device_alloc("sis5595", address);
+	pdev = platform_device_alloc(DRIVER_NAME, address);
 	if (!pdev) {
 		err = -ENOMEM;
 		pr_err("Device allocation failed\n");
@@ -886,7 +887,7 @@ static int sis5595_pci_probe(struct pci_dev *dev,
 }
 
 static struct pci_driver sis5595_pci_driver = {
-	.name            = "sis5595",
+	.name            = DRIVER_NAME,
 	.id_table        = sis5595_pci_ids,
 	.probe           = sis5595_pci_probe,
 };

base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
-- 
2.37.2

[PATCH 2/2] hwmon: sis5595: Reorder symbols to get rid of a few forward declarations

[Uwe Kleine-König]

Declarations for static symbols are useless repetition unless there are
cyclic dependencies.

Reorder the functions and variables to get rid of 6 forward declarations.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwmon/sis5595.c | 176 +++++++++++++++++++---------------------
 1 file changed, 84 insertions(+), 92 deletions(-)

diff --git a/drivers/hwmon/sis5595.c b/drivers/hwmon/sis5595.c
index 013f87da6fff..b0b05fd12221 100644
--- a/drivers/hwmon/sis5595.c
+++ b/drivers/hwmon/sis5595.c
@@ -192,21 +192,75 @@ struct sis5595_data {
 
 static struct pci_dev *s_bridge;	/* pointer to the (only) sis5595 */
 
-static int sis5595_probe(struct platform_device *pdev);
-static int sis5595_remove(struct platform_device *pdev);
+/* ISA access must be locked explicitly. */
+static int sis5595_read_value(struct sis5595_data *data, u8 reg)
+{
+	int res;
 
-static int sis5595_read_value(struct sis5595_data *data, u8 reg);
-static void sis5595_write_value(struct sis5595_data *data, u8 reg, u8 value);
-static struct sis5595_data *sis5595_update_device(struct device *dev);
-static void sis5595_init_device(struct sis5595_data *data);
+	mutex_lock(&data->lock);
+	outb_p(reg, data->addr + SIS5595_ADDR_REG_OFFSET);
+	res = inb_p(data->addr + SIS5595_DATA_REG_OFFSET);
+	mutex_unlock(&data->lock);
+	return res;
+}
 
-static struct platform_driver sis5595_driver = {
-	.driver = {
-		.name	= DRIVER_NAME,
-	},
-	.probe		= sis5595_probe,
-	.remove		= sis5595_remove,
-};
+static void sis5595_write_value(struct sis5595_data *data, u8 reg, u8 value)
+{
+	mutex_lock(&data->lock);
+	outb_p(reg, data->addr + SIS5595_ADDR_REG_OFFSET);
+	outb_p(value, data->addr + SIS5595_DATA_REG_OFFSET);
+	mutex_unlock(&data->lock);
+}
+
+static struct sis5595_data *sis5595_update_device(struct device *dev)
+{
+	struct sis5595_data *data = dev_get_drvdata(dev);
+	int i;
+
+	mutex_lock(&data->update_lock);
+
+	if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
+	    || !data->valid) {
+
+		for (i = 0; i <= data->maxins; i++) {
+			data->in[i] =
+			    sis5595_read_value(data, SIS5595_REG_IN(i));
+			data->in_min[i] =
+			    sis5595_read_value(data,
+					       SIS5595_REG_IN_MIN(i));
+			data->in_max[i] =
+			    sis5595_read_value(data,
+					       SIS5595_REG_IN_MAX(i));
+		}
+		for (i = 0; i < 2; i++) {
+			data->fan[i] =
+			    sis5595_read_value(data, SIS5595_REG_FAN(i));
+			data->fan_min[i] =
+			    sis5595_read_value(data,
+					       SIS5595_REG_FAN_MIN(i));
+		}
+		if (data->maxins == 3) {
+			data->temp =
+			    sis5595_read_value(data, SIS5595_REG_TEMP);
+			data->temp_over =
+			    sis5595_read_value(data, SIS5595_REG_TEMP_OVER);
+			data->temp_hyst =
+			    sis5595_read_value(data, SIS5595_REG_TEMP_HYST);
+		}
+		i = sis5595_read_value(data, SIS5595_REG_FANDIV);
+		data->fan_div[0] = (i >> 4) & 0x03;
+		data->fan_div[1] = i >> 6;
+		data->alarms =
+		    sis5595_read_value(data, SIS5595_REG_ALARM1) |
+		    (sis5595_read_value(data, SIS5595_REG_ALARM2) << 8);
+		data->last_updated = jiffies;
+		data->valid = true;
+	}
+
+	mutex_unlock(&data->update_lock);
+
+	return data;
+}
 
 /* 4 Voltages */
 static ssize_t in_show(struct device *dev, struct device_attribute *da,
@@ -569,6 +623,15 @@ static const struct attribute_group sis5595_group_temp1 = {
 	.attrs = sis5595_attributes_temp1,
 };
 
+/* Called when we have found a new SIS5595. */
+static void sis5595_init_device(struct sis5595_data *data)
+{
+	u8 config = sis5595_read_value(data, SIS5595_REG_CONFIG);
+	if (!(config & 0x01))
+		sis5595_write_value(data, SIS5595_REG_CONFIG,
+				(config & 0xf7) | 0x01);
+}
+
 /* This is called when the module is loaded */
 static int sis5595_probe(struct platform_device *pdev)
 {
@@ -658,85 +721,6 @@ static int sis5595_remove(struct platform_device *pdev)
 	return 0;
 }
 
-/* ISA access must be locked explicitly. */
-static int sis5595_read_value(struct sis5595_data *data, u8 reg)
-{
-	int res;
-
-	mutex_lock(&data->lock);
-	outb_p(reg, data->addr + SIS5595_ADDR_REG_OFFSET);
-	res = inb_p(data->addr + SIS5595_DATA_REG_OFFSET);
-	mutex_unlock(&data->lock);
-	return res;
-}
-
-static void sis5595_write_value(struct sis5595_data *data, u8 reg, u8 value)
-{
-	mutex_lock(&data->lock);
-	outb_p(reg, data->addr + SIS5595_ADDR_REG_OFFSET);
-	outb_p(value, data->addr + SIS5595_DATA_REG_OFFSET);
-	mutex_unlock(&data->lock);
-}
-
-/* Called when we have found a new SIS5595. */
-static void sis5595_init_device(struct sis5595_data *data)
-{
-	u8 config = sis5595_read_value(data, SIS5595_REG_CONFIG);
-	if (!(config & 0x01))
-		sis5595_write_value(data, SIS5595_REG_CONFIG,
-				(config & 0xf7) | 0x01);
-}
-
-static struct sis5595_data *sis5595_update_device(struct device *dev)
-{
-	struct sis5595_data *data = dev_get_drvdata(dev);
-	int i;
-
-	mutex_lock(&data->update_lock);
-
-	if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
-	    || !data->valid) {
-
-		for (i = 0; i <= data->maxins; i++) {
-			data->in[i] =
-			    sis5595_read_value(data, SIS5595_REG_IN(i));
-			data->in_min[i] =
-			    sis5595_read_value(data,
-					       SIS5595_REG_IN_MIN(i));
-			data->in_max[i] =
-			    sis5595_read_value(data,
-					       SIS5595_REG_IN_MAX(i));
-		}
-		for (i = 0; i < 2; i++) {
-			data->fan[i] =
-			    sis5595_read_value(data, SIS5595_REG_FAN(i));
-			data->fan_min[i] =
-			    sis5595_read_value(data,
-					       SIS5595_REG_FAN_MIN(i));
-		}
-		if (data->maxins == 3) {
-			data->temp =
-			    sis5595_read_value(data, SIS5595_REG_TEMP);
-			data->temp_over =
-			    sis5595_read_value(data, SIS5595_REG_TEMP_OVER);
-			data->temp_hyst =
-			    sis5595_read_value(data, SIS5595_REG_TEMP_HYST);
-		}
-		i = sis5595_read_value(data, SIS5595_REG_FANDIV);
-		data->fan_div[0] = (i >> 4) & 0x03;
-		data->fan_div[1] = i >> 6;
-		data->alarms =
-		    sis5595_read_value(data, SIS5595_REG_ALARM1) |
-		    (sis5595_read_value(data, SIS5595_REG_ALARM2) << 8);
-		data->last_updated = jiffies;
-		data->valid = true;
-	}
-
-	mutex_unlock(&data->update_lock);
-
-	return data;
-}
-
 static const struct pci_device_id sis5595_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_503) },
 	{ 0, }
@@ -801,6 +785,14 @@ static int sis5595_device_add(unsigned short address)
 	return err;
 }
 
+static struct platform_driver sis5595_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+	},
+	.probe		= sis5595_probe,
+	.remove		= sis5595_remove,
+};
+
 static int sis5595_pci_probe(struct pci_dev *dev,
 				       const struct pci_device_id *id)
 {
-- 
2.37.2

Re: [PATCH 1/2] hwmon: sis5595: Introduce a #define for the driver name and use it

[Guenter Roeck]

On Thu, Sep 22, 2022 at 09:48:59AM +0200, Uwe Kleine-König wrote:
> Make use of the cpp symbol DRIVER_NAME to set the driver's name and use
> it instead of all explicit usages of the same string. Also make use of
> it instead of sis5595_driver.driver.name which breaks a cyclic dependency
> between sis5595_probe() and sis5595_driver that in the next commit allows
> to drop some forward declarations.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied to hwmon-next.

Thanks,
Guenter

Re: [PATCH 2/2] hwmon: sis5595: Reorder symbols to get rid of a few forward declarations

[Guenter Roeck]

On Thu, Sep 22, 2022 at 09:49:00AM +0200, Uwe Kleine-König wrote:
> Declarations for static symbols are useless repetition unless there are
> cyclic dependencies.
> 
> Reorder the functions and variables to get rid of 6 forward declarations.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied to hwmon-next.

Thanks,
Guenter

Re: [PATCH 2/2] hwmon: sis5595: Reorder symbols to get rid of a few forward declarations

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

Hello Guenter,

On Sat, Sep 24, 2022 at 07:37:29AM -0700, Guenter Roeck wrote:
> On Sat, Sep 24, 2022 at 03:57:38PM +0200, Uwe Kleine-König wrote:
> > Declarations for static symbols are useless repetition unless there are
> > cyclic dependencies.
> > 
> > Reorder the functions and variables to get rid of 4 forward declarations.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Applied, after fixing up subject (via686a, not sis5595).

When I pasted that into citool I had the plan to fix that ...

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] hwmon: sis5595: Reorder symbols to get rid of a few forward declarations

[Guenter Roeck]

On Sat, Sep 24, 2022 at 03:57:38PM +0200, Uwe Kleine-König wrote:
> Declarations for static symbols are useless repetition unless there are
> cyclic dependencies.
> 
> Reorder the functions and variables to get rid of 4 forward declarations.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied, after fixing up subject (via686a, not sis5595).

Thanks,
Guenter

[PATCH 2/2] hwmon: sis5595: Reorder symbols to get rid of a few forward declarations

[Uwe Kleine-König]

Declarations for static symbols are useless repetition unless there are
cyclic dependencies.

Reorder the functions and variables to get rid of 4 forward declarations.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwmon/via686a.c | 194 +++++++++++++++++++---------------------
 1 file changed, 94 insertions(+), 100 deletions(-)

diff --git a/drivers/hwmon/via686a.c b/drivers/hwmon/via686a.c
index b17121881235..37d7374896f6 100644
--- a/drivers/hwmon/via686a.c
+++ b/drivers/hwmon/via686a.c
@@ -323,9 +323,6 @@ struct via686a_data {
 
 static struct pci_dev *s_bridge;	/* pointer to the (only) via686a */
 
-static int via686a_probe(struct platform_device *pdev);
-static int via686a_remove(struct platform_device *pdev);
-
 static inline int via686a_read_value(struct via686a_data *data, u8 reg)
 {
 	return inb_p(data->addr + reg);
@@ -337,8 +334,76 @@ static inline void via686a_write_value(struct via686a_data *data, u8 reg,
 	outb_p(value, data->addr + reg);
 }
 
-static struct via686a_data *via686a_update_device(struct device *dev);
-static void via686a_init_device(struct via686a_data *data);
+static void via686a_update_fan_div(struct via686a_data *data)
+{
+	int reg = via686a_read_value(data, VIA686A_REG_FANDIV);
+	data->fan_div[0] = (reg >> 4) & 0x03;
+	data->fan_div[1] = reg >> 6;
+}
+
+static struct via686a_data *via686a_update_device(struct device *dev)
+{
+	struct via686a_data *data = dev_get_drvdata(dev);
+	int i;
+
+	mutex_lock(&data->update_lock);
+
+	if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
+	    || !data->valid) {
+		for (i = 0; i <= 4; i++) {
+			data->in[i] =
+			    via686a_read_value(data, VIA686A_REG_IN(i));
+			data->in_min[i] = via686a_read_value(data,
+							     VIA686A_REG_IN_MIN
+							     (i));
+			data->in_max[i] =
+			    via686a_read_value(data, VIA686A_REG_IN_MAX(i));
+		}
+		for (i = 1; i <= 2; i++) {
+			data->fan[i - 1] =
+			    via686a_read_value(data, VIA686A_REG_FAN(i));
+			data->fan_min[i - 1] = via686a_read_value(data,
+						     VIA686A_REG_FAN_MIN(i));
+		}
+		for (i = 0; i <= 2; i++) {
+			data->temp[i] = via686a_read_value(data,
+						 VIA686A_REG_TEMP[i]) << 2;
+			data->temp_over[i] =
+			    via686a_read_value(data,
+					       VIA686A_REG_TEMP_OVER[i]);
+			data->temp_hyst[i] =
+			    via686a_read_value(data,
+					       VIA686A_REG_TEMP_HYST[i]);
+		}
+		/*
+		 * add in lower 2 bits
+		 * temp1 uses bits 7-6 of VIA686A_REG_TEMP_LOW1
+		 * temp2 uses bits 5-4 of VIA686A_REG_TEMP_LOW23
+		 * temp3 uses bits 7-6 of VIA686A_REG_TEMP_LOW23
+		 */
+		data->temp[0] |= (via686a_read_value(data,
+						     VIA686A_REG_TEMP_LOW1)
+				  & 0xc0) >> 6;
+		data->temp[1] |=
+		    (via686a_read_value(data, VIA686A_REG_TEMP_LOW23) &
+		     0x30) >> 4;
+		data->temp[2] |=
+		    (via686a_read_value(data, VIA686A_REG_TEMP_LOW23) &
+		     0xc0) >> 6;
+
+		via686a_update_fan_div(data);
+		data->alarms =
+		    via686a_read_value(data,
+				       VIA686A_REG_ALARM1) |
+		    (via686a_read_value(data, VIA686A_REG_ALARM2) << 8);
+		data->last_updated = jiffies;
+		data->valid = true;
+	}
+
+	mutex_unlock(&data->update_lock);
+
+	return data;
+}
 
 /* following are the sysfs callback functions */
 
@@ -656,13 +721,23 @@ static const struct attribute_group via686a_group = {
 	.attrs = via686a_attributes,
 };
 
-static struct platform_driver via686a_driver = {
-	.driver = {
-		.name	= DRIVER_NAME,
-	},
-	.probe		= via686a_probe,
-	.remove		= via686a_remove,
-};
+static void via686a_init_device(struct via686a_data *data)
+{
+	u8 reg;
+
+	/* Start monitoring */
+	reg = via686a_read_value(data, VIA686A_REG_CONFIG);
+	via686a_write_value(data, VIA686A_REG_CONFIG, (reg | 0x01) & 0x7F);
+
+	/* Configure temp interrupt mode for continuous-interrupt operation */
+	reg = via686a_read_value(data, VIA686A_REG_TEMP_MODE);
+	via686a_write_value(data, VIA686A_REG_TEMP_MODE,
+			    (reg & ~VIA686A_TEMP_MODE_MASK)
+			    | VIA686A_TEMP_MODE_CONTINUOUS);
+
+	/* Pre-read fan clock divisor values */
+	via686a_update_fan_div(data);
+}
 
 /* This is called when the module is loaded */
 static int via686a_probe(struct platform_device *pdev)
@@ -721,94 +796,13 @@ static int via686a_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static void via686a_update_fan_div(struct via686a_data *data)
-{
-	int reg = via686a_read_value(data, VIA686A_REG_FANDIV);
-	data->fan_div[0] = (reg >> 4) & 0x03;
-	data->fan_div[1] = reg >> 6;
-}
-
-static void via686a_init_device(struct via686a_data *data)
-{
-	u8 reg;
-
-	/* Start monitoring */
-	reg = via686a_read_value(data, VIA686A_REG_CONFIG);
-	via686a_write_value(data, VIA686A_REG_CONFIG, (reg | 0x01) & 0x7F);
-
-	/* Configure temp interrupt mode for continuous-interrupt operation */
-	reg = via686a_read_value(data, VIA686A_REG_TEMP_MODE);
-	via686a_write_value(data, VIA686A_REG_TEMP_MODE,
-			    (reg & ~VIA686A_TEMP_MODE_MASK)
-			    | VIA686A_TEMP_MODE_CONTINUOUS);
-
-	/* Pre-read fan clock divisor values */
-	via686a_update_fan_div(data);
-}
-
-static struct via686a_data *via686a_update_device(struct device *dev)
-{
-	struct via686a_data *data = dev_get_drvdata(dev);
-	int i;
-
-	mutex_lock(&data->update_lock);
-
-	if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
-	    || !data->valid) {
-		for (i = 0; i <= 4; i++) {
-			data->in[i] =
-			    via686a_read_value(data, VIA686A_REG_IN(i));
-			data->in_min[i] = via686a_read_value(data,
-							     VIA686A_REG_IN_MIN
-							     (i));
-			data->in_max[i] =
-			    via686a_read_value(data, VIA686A_REG_IN_MAX(i));
-		}
-		for (i = 1; i <= 2; i++) {
-			data->fan[i - 1] =
-			    via686a_read_value(data, VIA686A_REG_FAN(i));
-			data->fan_min[i - 1] = via686a_read_value(data,
-						     VIA686A_REG_FAN_MIN(i));
-		}
-		for (i = 0; i <= 2; i++) {
-			data->temp[i] = via686a_read_value(data,
-						 VIA686A_REG_TEMP[i]) << 2;
-			data->temp_over[i] =
-			    via686a_read_value(data,
-					       VIA686A_REG_TEMP_OVER[i]);
-			data->temp_hyst[i] =
-			    via686a_read_value(data,
-					       VIA686A_REG_TEMP_HYST[i]);
-		}
-		/*
-		 * add in lower 2 bits
-		 * temp1 uses bits 7-6 of VIA686A_REG_TEMP_LOW1
-		 * temp2 uses bits 5-4 of VIA686A_REG_TEMP_LOW23
-		 * temp3 uses bits 7-6 of VIA686A_REG_TEMP_LOW23
-		 */
-		data->temp[0] |= (via686a_read_value(data,
-						     VIA686A_REG_TEMP_LOW1)
-				  & 0xc0) >> 6;
-		data->temp[1] |=
-		    (via686a_read_value(data, VIA686A_REG_TEMP_LOW23) &
-		     0x30) >> 4;
-		data->temp[2] |=
-		    (via686a_read_value(data, VIA686A_REG_TEMP_LOW23) &
-		     0xc0) >> 6;
-
-		via686a_update_fan_div(data);
-		data->alarms =
-		    via686a_read_value(data,
-				       VIA686A_REG_ALARM1) |
-		    (via686a_read_value(data, VIA686A_REG_ALARM2) << 8);
-		data->last_updated = jiffies;
-		data->valid = true;
-	}
-
-	mutex_unlock(&data->update_lock);
-
-	return data;
-}
+static struct platform_driver via686a_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+	},
+	.probe		= via686a_probe,
+	.remove		= via686a_remove,
+};
 
 static const struct pci_device_id via686a_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4) },
-- 
2.37.2