On Sat, Nov 05, 2022 at 03:57:54PM +1100, Frank Crawford wrote:
Description got lost, and the change log should be after '---'.
Otherwise looks good.
> Changes in this patch set:
>
> * 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.
>
> v2: updates following comments:
>
> * Converted to use callback function.
>
> * Moved call to callback funtion to sio_data into it87_find in line
> with other settings for sio_data. This requires dmi_data also passed
> to access additional data.
>
> * Added macro for defining entries in DMI table to simplify future
> additions.
>
> * Note dmi_data is defined in sm_it87_init to simplify tests and for
> future additions.
>
> v3: further updates following comments:
>
> * Proper use of callback functions for DMI functions. This also
> involves saving dmi_data in a static variable for use as required.
>
> * Moved to dmi_check_system() for testing DMI table.
>
> v4: further cleanups following comments
>
> * Note the macro IT87_DMI_MATCH_VND includes callback argument for use
> by a future update.
>
> Signed-off-by: Frank Crawford <frank@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/hwmon/it87.c | 72 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 53 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 73ed21ab325b..9997f76b1f4a 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -567,6 +567,14 @@ struct it87_data {
> s8 auto_temp[NUM_AUTO_PWM][5]; /* [nr][0] is point1_temp_hyst */
> };
>
> +/* Board specific settings from DMI matching */
> +struct it87_dmi_data {
> + u8 skip_pwm; /* pwm channels to skip for this board */
> +};
> +
> +/* Global for results from DMI matching, if needed */
> +static struct it87_dmi_data *dmi_data;
> +
> static int adc_lsb(const struct it87_data *data, int nr)
> {
> int lsb;
> @@ -2393,7 +2401,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);
> @@ -2812,24 +2819,9 @@ 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");
> - sio_data->skip_pwm = BIT(1);
> - }
> - }
> + /* Set values based on DMI matches */
> + if (dmi_data)
> + sio_data->skip_pwm |= dmi_data->skip_pwm;
>
> exit:
> superio_exit(sioaddr);
> @@ -3307,6 +3299,46 @@ static int __init it87_device_add(int index, unsigned short address,
> return err;
> }
>
> +/* callback function for DMI */
> +static int it87_dmi_cb(const struct dmi_system_id *dmi_entry)
> +{
> + dmi_data = dmi_entry->driver_data;
> +
> + if (dmi_data && dmi_data->skip_pwm)
> + pr_info("Disabling pwm2 due to hardware constraints\n");
> +
> + return 1;
> +}
> +
> +/*
> + * 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),
> +};
> +
> +#define IT87_DMI_MATCH_VND(vendor, name, cb, data) \
> + { \
> + .callback = cb, \
> + .matches = { \
> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, vendor), \
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, name), \
> + }, \
> + .driver_data = data, \
> + }
> +
> +static const struct dmi_system_id it87_dmi_table[] __initconst = {
> + IT87_DMI_MATCH_VND("nVIDIA", "FN68PT", it87_dmi_cb, &nvidia_fn68pt),
> + { }
> +
> +};
> +MODULE_DEVICE_TABLE(dmi, it87_dmi_table);
> +
> static int __init sm_it87_init(void)
> {
> int sioaddr[2] = { REG_2E, REG_4E };
> @@ -3319,6 +3351,8 @@ static int __init sm_it87_init(void)
> if (err)
> return err;
>
> + dmi_check_system(it87_dmi_table);
> +
> for (i = 0; i < ARRAY_SIZE(sioaddr); i++) {
> memset(&sio_data, 0, sizeof(struct it87_sio_data));
> isa_address[i] = 0;
> --
> 2.38.1
>
