| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <57B6C9FD.2010709@topic.nl>
Date: Fri, 19 Aug 2016 10:57:33 +0200
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: Guenter Roeck <linux@...ck-us.net>, hjk@...sjkoch.de
Cc: linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] hwmon: (max6650.c) Add devicetree support
(resent through different SMTP server, bounced as spam because of company
server's unwanted modifications, sorry if you received this twice now)
On 19-08-16 04:43, Guenter Roeck wrote:
> On 08/16/2016 12:20 AM, Mike Looijmans wrote:
>> Parse devicetree parameters for voltage and prescaler setting. This allows
>> using multiple max6550 devices with varying settings, and also makes it
>> possible to instantiate and configure the device using devicetree.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
>> ---
>> v4: Vendor prefix "maxim," for devicetree properties and compatible string
>> Split documentation into separate patch
>> v3: Resubmit because wrong mailing lists used
>> Fix style errors as reported by checkpatch.pl
>> Fix bug in DT parsing of fan-prescale
>> v2: Add devicetree binding documentation
>> Code changes as suggested by Guenter
>> Reduce log info, output only a single line
>> drivers/hwmon/max6650.c | 47 ++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 36 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
>> index 162a520..6beb62c 100644
>> --- a/drivers/hwmon/max6650.c
>> +++ b/drivers/hwmon/max6650.c
>> @@ -39,6 +39,7 @@
>> #include <linux/hwmon.h>
>> #include <linux/hwmon-sysfs.h>
>> #include <linux/err.h>
>> +#include <linux/of_device.h>
>>
>> /*
>> * Insmod parameters
>> @@ -48,7 +49,7 @@
>> static int fan_voltage;
>> /* prescaler: Possible values are 1, 2, 4, 8, 16 or 0 for don't change */
>> static int prescaler;
>> -/* clock: The clock frequency of the chip the driver should assume */
>> +/* clock: The clock frequency of the chip (max6651 can be clocked
>> externally) */
>> static int clock = 254000;
>>
>> module_param(fan_voltage, int, S_IRUGO);
>> @@ -133,6 +134,19 @@ static const u8 tach_reg[] = {
>> MAX6650_REG_TACH3,
>> };
>>
>> +static const struct of_device_id max6650_dt_match[] = {
>> + {
>> + .compatible = "maxim,max6650",
>> + .data = (void *)1
>> + },
>> + {
>> + .compatible = "maxim,max6651",
>> + .data = (void *)4
>> + },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, max6650_dt_match);
>> +
>> static struct max6650_data *max6650_update_device(struct device *dev)
>> {
>> struct max6650_data *data = dev_get_drvdata(dev);
>> @@ -566,6 +580,17 @@ static int max6650_init_client(struct max6650_data *data,
>> struct device *dev = &client->dev;
>> int config;
>> int err = -EIO;
>> + u32 voltage;
>> + u32 prescale;
>> +
>> + if (of_property_read_u32(dev->of_node, "maxim,fan-microvolt",
>> + &voltage))
>> + voltage = fan_voltage;
>> + else
>> + voltage = voltage > 5500000 ? 12 : 5;
>
> I think this should only accept 5V or 12V.
>
> Guenter
A 4V, 10V or 13V fan will also work just fine, the voltage register just sets
the range of the feedback tachometer.
I don't have strong feelings either way, so if you feel the drive should bail
out on anything but 5 or 12, that's fine with me too.
Should I change this and send a v5 patch?
>> + if (of_property_read_u32(dev->of_node, "maxim,fan-prescale",
>> + &prescale))
>> + prescale = prescaler;
>>
>> config = i2c_smbus_read_byte_data(client, MAX6650_REG_CONFIG);
>>
>> @@ -574,7 +599,7 @@ static int max6650_init_client(struct max6650_data *data,
>> return err;
>> }
>>
>> - switch (fan_voltage) {
>> + switch (voltage) {
>> case 0:
>> break;
>> case 5:
>> @@ -584,14 +609,10 @@ static int max6650_init_client(struct max6650_data *data,
>> config |= MAX6650_CFG_V12;
>> break;
>> default:
>> - dev_err(dev, "illegal value for fan_voltage (%d)\n",
>> - fan_voltage);
>> + dev_err(dev, "illegal value for fan_voltage (%d)\n", voltage);
>> }
>>
>> - dev_info(dev, "Fan voltage is set to %dV.\n",
>> - (config & MAX6650_CFG_V12) ? 12 : 5);
>> -
>> - switch (prescaler) {
>> + switch (prescale) {
>> case 0:
>> break;
>> case 1:
>> @@ -614,10 +635,11 @@ static int max6650_init_client(struct max6650_data *data,
>> | MAX6650_CFG_PRESCALER_16;
>> break;
>> default:
>> - dev_err(dev, "illegal value for prescaler (%d)\n", prescaler);
>> + dev_err(dev, "illegal value for prescaler (%d)\n", prescale);
>> }
>>
>> - dev_info(dev, "Prescaler is set to %d.\n",
>> + dev_info(dev, "Fan voltage: %dV, prescaler: %d.\n",
>> + (config & MAX6650_CFG_V12) ? 12 : 5,
>> 1 << (config & MAX6650_CFG_PRESCALER_MASK));
>>
>> /*
>> @@ -651,6 +673,8 @@ static int max6650_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> struct device *dev = &client->dev;
>> + const struct of_device_id *of_id =
>> + of_match_device(of_match_ptr(max6650_dt_match), dev);
>> struct max6650_data *data;
>> struct device *hwmon_dev;
>> int err;
>> @@ -661,7 +685,7 @@ static int max6650_probe(struct i2c_client *client,
>>
>> data->client = client;
>> mutex_init(&data->update_lock);
>> - data->nr_fans = id->driver_data;
>> + data->nr_fans = of_id ? (int)(uintptr_t)of_id->data : id->driver_data;
>>
>> /*
>> * Initialize the max6650 chip
>> @@ -691,6 +715,7 @@ MODULE_DEVICE_TABLE(i2c, max6650_id);
>> static struct i2c_driver max6650_driver = {
>> .driver = {
>> .name = "max6650",
>> + .of_match_table = of_match_ptr(max6650_dt_match),
>> },
>> .probe = max6650_probe,
>> .id_table = max6650_id,
>>
>
Powered by blists - more mailing lists