linux-kernel - Re: [RFC] hwmon: ina2xx: port to using remap, improve bandwidth.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 Oct 2015 19:21:42 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Marc Titinger <mtitinger@...libre.com>, jdelvare@...e.com
Cc:	lm-sensors@...sensors.org, linux-kernel@...r.kernel.org,
	mturquette@...libre.com, bcousson@...libre.com
Subject: Re: [RFC] hwmon: ina2xx: port to using remap, improve bandwidth.

On 10/23/2015 01:35 PM, Marc Titinger wrote:
> Hi Guenter
>
> thanks for the review, answers bellow.
>
> Marc.
>

[ ... ]

>>> -    /*
>>> -     * Ina226 has a variable update_interval. For ina219 we
>>> -     * use a constant value.
>>> +    /* Check for shunt resistor value.

Another comment: Standard multi-line comments, please.

>>> +     * Give precedence to device tree over must-recompile.
>>>        */
>>> -    if (data->kind == ina226)
>>> -        ina226_set_update_interval(data);
>>> -    else
>>> -        data->update_interval = HZ / INA2XX_CONVERSION_RATE;
>>> +    if (of_property_read_u32(dev->of_node, "shunt-resistor", &val) < 0) {
>>> +        pdata = dev_get_platdata(dev);
>>> +        if (pdata)
>>> +            val = pdata->shunt_uohms;
>>> +        else
>>> +            val = INA2XX_RSHUNT_DEFAULT;
>>> +    }
>>
>> This changes priority from platform data first to devicetree configuration first.
>> As such, it is an unrelated change. If needed, split into a separate patch, and
> Yes I would do a separate patch normaly, agreed.
>
>> explain the reasoning, please.
> Changing the platform data requires changes in the kernel code, and hence recompilation. It seems a bit unexpected that setting a new value in the dtb will be ignored because there is a compiled-in platform data. Should'nt the dtb allow to override platform data ?

Normally you would not _have_ platform data in a system which is dtb enabled.
I don't really mind changing priorities (you are right, it makes sense to
check for devicetree data first), but as mentioned as separate patch, please.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/