Re: [PATCH 1/1] hwmon: (it86) Create DMI matching table for various board settings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/15/22 05:01, Frank Crawford wrote:
Define the DMI matching table for board specific settings during the
chip initialisation and move the only current board specific setting
to this new table.

Export the table for use by udev.

Signed-off-by: Frank Crawford <frank@xxxxxxxxxxxxxxxxxx>
---
  drivers/hwmon/it87.c | 57 ++++++++++++++++++++++++++++----------------
  1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 7bd154ba351b..b83ef7c89095 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -2389,7 +2389,6 @@ static int __init it87_find(int sioaddr, unsigned short *address,
  {
  	int err;
  	u16 chip_type;
-	const char *board_vendor, *board_name;
  	const struct it87_devices *config;
err = superio_enter(sioaddr);
@@ -2802,25 +2801,6 @@ static int __init it87_find(int sioaddr, unsigned short *address,
  	if (sio_data->beep_pin)
  		pr_info("Beeping is supported\n");
- /* Disable specific features based on DMI strings */
-	board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
-	board_name = dmi_get_system_info(DMI_BOARD_NAME);
-	if (board_vendor && board_name) {
-		if (strcmp(board_vendor, "nVIDIA") == 0 &&
-		    strcmp(board_name, "FN68PT") == 0) {
-			/*
-			 * On the Shuttle SN68PT, FAN_CTL2 is apparently not
-			 * connected to a fan, but to something else. One user
-			 * has reported instant system power-off when changing
-			 * the PWM2 duty cycle, so we disable it.
-			 * I use the board name string as the trigger in case
-			 * the same board is ever used in other systems.
-			 */
-			pr_info("Disabling pwm2 due to hardware constraints\n");

This info message gets lost.

-			sio_data->skip_pwm = BIT(1);
-		}
-	}
-
  exit:
  	superio_exit(sioaddr);
  	return err;
@@ -3295,14 +3275,48 @@ static int __init it87_device_add(int index, unsigned short address,
  	return err;
  }
+struct it87_dmi_data {
+	u8 skip_pwm;		/* pwm channels to skip for this board  */
+};
+
+/*
+ * On the Shuttle SN68PT, FAN_CTL2 is apparently not
+ * connected to a fan, but to something else. One user
+ * has reported instant system power-off when changing
+ * the PWM2 duty cycle, so we disable it.
+ * I use the board name string as the trigger in case
+ * the same board is ever used in other systems.
+ */
+static struct it87_dmi_data nvidia_fn68pt = {
+	.skip_pwm = BIT(1),
+};
+
+static const struct dmi_system_id it87_dmi_table[] __initconst = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "nVIDIA"),
+			DMI_MATCH(DMI_BOARD_NAME, "FN68PT"),
+		},
+		.driver_data = &nvidia_fn68pt,

Maybe add a callback function and have it display the info message.
Using a callback function may make it easier to add functionality
needed for other boards. Hard to say, though, since it depends on
what is coming next.

+	},
+	{ }
+
+};
+MODULE_DEVICE_TABLE(dmi, it87_dmi_table);
+
  static int __init sm_it87_init(void)
  {
+	const struct dmi_system_id *dmi = dmi_first_match(it87_dmi_table);
+	struct it87_dmi_data *dmi_data = NULL;
  	int sioaddr[2] = { REG_2E, REG_4E };
  	struct it87_sio_data sio_data;
  	unsigned short isa_address[2];
  	bool found = false;
  	int i, err;
+ if (dmi)
+		dmi_data = dmi->driver_data;
+

dmi_data is really unnecessary. Might just check for dmi below and
dereference there if not NULL.

  	err = platform_driver_register(&it87_driver);
  	if (err)
  		return err;
@@ -3320,6 +3334,9 @@ static int __init sm_it87_init(void)
  		if (i && isa_address[i] == isa_address[0])
  			break;
+ if (dmi_data)
+			sio_data.skip_pwm |= dmi_data->skip_pwm;
+

Personally I would prefer a solution which does not require extra code here,
but I don't know a good one. The only means for the callback function to return
data to here seems to be through static variables. Any idea what else is coming ?
One option might be to keep a global copy of sio_data (eg it87_sio_data),
initialize it from the dmi callback function, and use
		sio_data = it87_sio_data;
instead of memset() to initialize it.

Thanks,
Guenter

  		err = it87_device_add(i, isa_address[i], &sio_data);
  		if (err)
  			goto exit_dev_unregister;




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux