linux-kernel - Re: [PATCH v3 11/19] hwmon: (mr75203) add VM pre-scaler support

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, 1 Sep 2022 17:17:40 +0300
From:   "Farber, Eliav" <farbere@...zon.com>
To:     Andy Shevchenko <andriy.shevchenko@...el.com>
CC:     <jdelvare@...e.com>, <linux@...ck-us.net>, <robh+dt@...nel.org>,
        <p.zabel@...gutronix.de>, <rtanwar@...linear.com>,
        <linux-hwmon@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <talel@...zon.com>,
        <hhhawa@...zon.com>, <jonnyc@...zon.com>, <hanochu@...zon.com>,
        <ronenk@...zon.com>, <itamark@...zon.com>, <shellykz@...zon.com>,
        <shorer@...zon.com>, <amitlavi@...zon.com>, <almogbs@...zon.com>,
        <dkl@...zon.com>, "Farber, Eliav" <farbere@...zon.com>
Subject: Re: [PATCH v3 11/19] hwmon: (mr75203) add VM pre-scaler support

On 8/31/2022 3:02 PM, Andy Shevchenko wrote:
> On Tue, Aug 30, 2022 at 07:22:04PM +0000, Eliav Farber wrote:
>> +static int pvt_get_pre_scaler(struct device *dev, struct pvt_device 
>> *pvt)
>> +{
>> +     const struct device_node *np = dev->of_node;
>> +     u32 total_channels = pvt->vm_channels.total;
>> +     u32 channel;
>> +     u8 *pre_scaler_ch_list;
>> +     int i, ret, num_ch;
>> +
>> +     /* Set default pre-scaler value to be 1. */
>> +     for (i = 0; i < total_channels; i++)
>> +             pvt->vd[i].pre_scaler = PRE_SCALER_X1;
>> +
>> +     /* Get number of channels configured in 
>> "moortec,vm-pre-scaler". */
>> +     num_ch = of_property_count_u8_elems(np, "moortec,vm-pre-scaler");
>
> of_ ?!
>
Replaced of_property_count_u8_elems() with
device_property_count_u8().

>> +     if (num_ch <= 0)
>> +             return 0;
>> +
>> +     pre_scaler_ch_list = kcalloc(total_channels,
>> +                                  sizeof(*pre_scaler_ch_list), 
>> GFP_KERNEL);
>> +     if (!pre_scaler_ch_list)
>> +             return -ENOMEM;
>> +
>> +     /* Get list of all channels that have pre-scaler of 2. */
>> +     ret = device_property_read_u8_array(dev, "moortec,vm-pre-scaler",
>> +                                         pre_scaler_ch_list, num_ch);
>> +     if (ret)
>> +             goto out;
>> +
>> +     for (i = 0; i < num_ch; i++) {
>> +             channel = pre_scaler_ch_list[i];
>
>> +
>
> Unnecessary blank line.

Blank line removed.


>> +             if (channel >= total_channels) {
>> +                     dev_err(dev,
>> +                             "invalid channel (%u) in pre-scaler 
>> list\n",
>> +                             channel);
>> +                     ret = -EINVAL;
>
>> +                     goto out;
>
> Wouldn't
>
>                        break;
>
> suffice? (I understand the point, up to you)
I prefer to exit the moment I detect a problem.
For now I can use a break but in the future someone else can add new code
in between that will set ret to 0 and instead of failing driver flow will
continue with incomplete pre-scaler value.
So I prefer keeping it as it.

--
Best regards, Eliav