linux-kernel - Re: [PATCH 2/4] iio: dpot-dac: DAC driver based on a digital potentiometer

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:   Thu, 20 Oct 2016 16:51:23 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Peter Meerwald-Stadler <pmeerw@...erw.net>
CC:     <linux-kernel@...r.kernel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        "Rob Herring" <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH 2/4] iio: dpot-dac: DAC driver based on a digital
 potentiometer

On 2016-10-20 16:08, Peter Meerwald-Stadler wrote:
>>>> +	u32 max_ohms;
>>>> +};
>>>> +
>>>> +static const struct iio_chan_spec dpot_dac_iio_channel = {
>>>> +	.type = IIO_VOLTAGE,
>>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
>>>> +			    | BIT(IIO_CHAN_INFO_SCALE),
>>>> +	.output = 1,
>>>> +};
>>>> +
>>>> +static int dpot_dac_read_raw(struct iio_dev *indio_dev,
>>>> +			     struct iio_chan_spec const *chan,
>>>> +			     int *val, int *val2, long mask)
>>>> +{
>>>> +	struct dpot_dac *dac = iio_priv(indio_dev);
>>>> +	int ret;
>>>> +	unsigned long long tmp;
>>>
>>> s64 (which is used to cast below)
>>
>> I assume that what you really want is to get rid of the cast, and
>> that you aren't really bothered if tmp is ull or s32, right? Given
>> that assumption, I'm just dropping the cast instead. The LL specifier
>> on the constant should promote *val anyway.
> 
> what I meant to pick upon was mixing
> s64 with unsigned long long with LL in the code that follows
> 
> anything simpler and more consistent would be good

Right, I should however note that I copied the pattern from the
industrialio-core.c:iio_format_value function. So, in my defense
I was consistent with that...

Cheers,
Peter