linux-kernel - Re: [PATCH v2 3/3] PCI: imx: Add support for i.MX8MQ

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, 14 Dec 2018 14:55:49 -0600
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Bjorn Helgaas <helgaas@...nel.org>,
        Andrey Smirnov <andrew.smirnov@...il.com>
Cc:     linux-pci@...r.kernel.org, Fabio Estevam <fabio.estevam@....com>,
        cphealy@...il.com, l.stach@...gutronix.de,
        Leonard Crestez <leonard.crestez@....com>,
        "A.s. Dong" <aisheng.dong@....com>,
        Richard Zhu <hongxing.zhu@....com>, linux-imx@....com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/3] PCI: imx: Add support for i.MX8MQ

Hi,

On 12/14/18 2:30 PM, Bjorn Helgaas wrote:
> [+cc Gustavo for fallthrough annotation]

> 
>> @@ -301,6 +312,7 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>>   
>>   	switch (imx6_pcie->variant) {
>>   	case IMX7D:
>> +	case IMX8MQ: /* FALLTHROUGH */
>>   		reset_control_assert(imx6_pcie->pciephy_reset);
>>   		reset_control_assert(imx6_pcie->apps_reset);
>>   		break;
> 
> I'm not an expert on fallthrough annotation (Gustavo, cc'd, is), but
> this looks wrong.  It's the IMX7D case that falls through, not the
> IMX8MQ case.
> 

Correct.

> The recent annotations added by Gustavo are at the point where the
> "break" would normally be, e.g.,
> 
>    case IMX7D:
>      /* fall through */                    <--- annotation
>    case IMX8MQ:
>      <code>
>      break;
> 
> But in this case there's actually no IMX7D-specific *code* there, so I
> suspect the annotation is unnecessary.  It's obvious that IMX7D and
> IMX8MQ are handled the same, so there's really no opportunity for the
> "forgotten break" mistake -Wimplicit-fallthrough is trying to find.
> 

Yep. That's correct. There is no need for those annotations in this patch.

> If we *do* want this annotation, we should spell it the same as
> Gustavo has been, i.e., "fall through".
> 

This is a matter of style. For -Wimplicit-fallthrough, "FALLTHROUGH" is 
as valid a "fall through".

Although, currently, there are 1997 instances of "fall through" vs 235 
of "FALLTHROUGH" in linux-next.

Thanks
--
Gustavo