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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux