* [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ"
@ 2018-12-16 23:09 Andrey Smirnov
2018-12-16 23:09 ` [PATCH 1/3] PCI: imx6: Fixup Kconfig and #endif placement Andrey Smirnov
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-16 23:09 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Andrey Smirnov, Bjorn Helgaas, Fabio Estevam, Chris Healy,
Lucas Stach, Leonard Crestez, A.s. Dong, Richard Zhu, linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
Lorenzo:
These are patches fixing things pointed out by Bjorn in [1] and [2] as
well as something I noticed while giving pci/next branch a try (#endif
placement). Hopefully this is at least somewhat helpful.
Thanks,
Andrey Smirnov
[1] lkml.kernel.org/r/20181214203828.GD20725@google.com
[2] lkml.kernel.org/r/20181214203042.GC20725@google.com
Andrey Smirnov (3):
PCI: imx6: Fixup Kconfig and #endif placement
PCI: imx6: Invert checks in imx6_pcie_reset_phy() and
imx6_setup_phy_mpll()
PCI: imx6: Make fallthrough comments more consistent
drivers/pci/controller/dwc/Kconfig | 4 ++--
drivers/pci/controller/dwc/pci-imx6.c | 29 ++++++++++++++++++---------
2 files changed, 21 insertions(+), 12 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] PCI: imx6: Fixup Kconfig and #endif placement
2018-12-16 23:09 [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Andrey Smirnov
@ 2018-12-16 23:09 ` Andrey Smirnov
2018-12-16 23:09 ` [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll() Andrey Smirnov
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-16 23:09 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Andrey Smirnov, Bjorn Helgaas, Fabio Estevam, Chris Healy,
Lucas Stach, Leonard Crestez, A.s. Dong, Richard Zhu, linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
Fixup Kconfig, which lost SOC_IMX7D during merge as well as move
"endif" to its original placement to only imx6q_pcie_abort_handler()
when building for AArch64
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: "A.s. Dong" <aisheng.dong@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: linux-imx@nxp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
drivers/pci/controller/dwc/Kconfig | 4 ++--
drivers/pci/controller/dwc/pci-imx6.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 0c8142529241..b5202ed78762 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -89,8 +89,8 @@ config PCI_EXYNOS
select PCIE_DW_HOST
config PCI_IMX6
- bool "Freescale i.MX6/7 PCIe controller"
- depends on SOC_IMX8MQ || SOC_IMX6Q || (ARM && COMPILE_TEST)
+ bool "Freescale i.MX6/7/8 PCIe controller"
+ depends on SOC_IMX8MQ || SOC_IMX6Q || SOC_IMX7D || (ARM && COMPILE_TEST)
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index e563ca9bdb78..4b7f638b8aff 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -311,6 +311,7 @@ static int imx6q_pcie_abort_handler(unsigned long addr,
return 1;
}
+#endif
static int imx6_pcie_attach_pd(struct device *dev)
{
@@ -389,7 +390,6 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
ret);
}
}
-#endif
static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
{
--
2.19.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll()
2018-12-16 23:09 [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Andrey Smirnov
2018-12-16 23:09 ` [PATCH 1/3] PCI: imx6: Fixup Kconfig and #endif placement Andrey Smirnov
@ 2018-12-16 23:09 ` Andrey Smirnov
2018-12-17 10:24 ` Leonard Crestez
2018-12-16 23:09 ` [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent Andrey Smirnov
2018-12-17 11:00 ` [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Lorenzo Pieralisi
3 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-16 23:09 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Andrey Smirnov, Bjorn Helgaas, Fabio Estevam, Chris Healy,
Lucas Stach, Leonard Crestez, A.s. Dong, Richard Zhu, linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
In order to avoid having potentially ever growing list of variants
that don't support methods implemented in imx6_pcie_reset_phy() and
imx6_setup_phy_mpll(), change logical checks in the to check for SoC's
that _do_ support what they implement. While at it, share the code via
a small helper function.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: "A.s. Dong" <aisheng.dong@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: linux-imx@nxp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
drivers/pci/controller/dwc/pci-imx6.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 4b7f638b8aff..59658577e81d 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -256,12 +256,18 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
return 0;
}
+static bool imx6_pcie_has_imx6_phy(struct imx6_pcie *imx6_pcie)
+{
+ return imx6_pcie->variant == IMX6Q ||
+ imx6_pcie->variant == IMX6SX ||
+ imx6_pcie->variant == IMX6QP;
+}
+
static void imx6_pcie_reset_phy(struct imx6_pcie *imx6_pcie)
{
u32 tmp;
- if (imx6_pcie->variant == IMX7D ||
- imx6_pcie->variant == IMX8MQ)
+ if (!imx6_pcie_has_imx6_phy(imx6_pcie))
return;
pcie_phy_read(imx6_pcie, PHY_RX_OVRD_IN_LO, &tmp);
@@ -637,8 +643,7 @@ static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
int mult, div;
u32 val;
- if (imx6_pcie->variant == IMX7D ||
- imx6_pcie->variant == IMX8MQ)
+ if (!imx6_pcie_has_imx6_phy(imx6_pcie))
return 0;
switch (phy_rate) {
--
2.19.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent
2018-12-16 23:09 [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Andrey Smirnov
2018-12-16 23:09 ` [PATCH 1/3] PCI: imx6: Fixup Kconfig and #endif placement Andrey Smirnov
2018-12-16 23:09 ` [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll() Andrey Smirnov
@ 2018-12-16 23:09 ` Andrey Smirnov
2018-12-17 13:58 ` Bjorn Helgaas
2018-12-17 11:00 ` [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Lorenzo Pieralisi
3 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-16 23:09 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Andrey Smirnov, Bjorn Helgaas, Fabio Estevam, Chris Healy,
Lucas Stach, Leonard Crestez, A.s. Dong, Richard Zhu, linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
Convert all fallthrough comments to say "fall through", as well as
modify their placement to the point where the "break" would normally
be.
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: "A.s. Dong" <aisheng.dong@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: linux-imx@nxp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
drivers/pci/controller/dwc/pci-imx6.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 59658577e81d..a0510e185d44 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -362,7 +362,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
switch (imx6_pcie->variant) {
case IMX7D:
- case IMX8MQ: /* FALLTHROUGH */
+ /* fall through */
+ case IMX8MQ:
reset_control_assert(imx6_pcie->pciephy_reset);
reset_control_assert(imx6_pcie->apps_reset);
break;
@@ -415,7 +416,8 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
break;
- case IMX6QP: /* FALLTHROUGH */
+ case IMX6QP:
+ /* fall through */
case IMX6Q:
/* power up core phy and enable ref clock */
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
@@ -607,7 +609,7 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6SX_GPR12_PCIE_RX_EQ_MASK,
IMX6SX_GPR12_PCIE_RX_EQ_2);
- /* FALLTHROUGH */
+ /* fall through */
default:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
@@ -731,7 +733,8 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
IMX6Q_GPR12_PCIE_CTL_2);
break;
case IMX7D:
- case IMX8MQ: /* FALLTHROUGH */
+ /* fall through */
+ case IMX8MQ:
reset_control_deassert(imx6_pcie->apps_reset);
break;
}
@@ -1076,7 +1079,8 @@ static int imx6_pcie_probe(struct platform_device *pdev)
}
break;
case IMX8MQ:
- case IMX7D: /* FALLTHROUGH */
+ /* fall through */
+ case IMX7D:
imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev,
"pciephy");
if (IS_ERR(imx6_pcie->pciephy_reset)) {
--
2.19.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll()
2018-12-16 23:09 ` [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll() Andrey Smirnov
@ 2018-12-17 10:24 ` Leonard Crestez
2018-12-17 17:11 ` Andrey Smirnov
0 siblings, 1 reply; 10+ messages in thread
From: Leonard Crestez @ 2018-12-17 10:24 UTC (permalink / raw)
To: Andrey Smirnov, Lorenzo Pieralisi, Lucas Stach, Stefan Agner
Cc: Bjorn Helgaas, Fabio Estevam, Chris Healy, Aisheng Dong,
Richard Zhu, dl-linux-imx, linux-arm-kernel, linux-kernel,
linux-pci
On 12/17/2018 1:09 AM, Andrey Smirnov wrote:
> In order to avoid having potentially ever growing list of variants
> that don't support methods implemented in imx6_pcie_reset_phy() and
> imx6_setup_phy_mpll(), change logical checks in the to check for SoC's
> that _do_ support what they implement. While at it, share the code via
> a small helper function.
>
> +static bool imx6_pcie_has_imx6_phy(struct imx6_pcie *imx6_pcie)
> +{
> + return imx6_pcie->variant == IMX6Q ||
> + imx6_pcie->variant == IMX6SX ||
> + imx6_pcie->variant == IMX6QP;
> +}
This would be an ideal match for adding a field inside drvdata, sadly
that was part of a series which stalled:
https://patchwork.kernel.org/patch/10712261/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ"
2018-12-16 23:09 [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Andrey Smirnov
` (2 preceding siblings ...)
2018-12-16 23:09 ` [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent Andrey Smirnov
@ 2018-12-17 11:00 ` Lorenzo Pieralisi
2018-12-17 17:18 ` Andrey Smirnov
3 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Pieralisi @ 2018-12-17 11:00 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Bjorn Helgaas, Fabio Estevam, Chris Healy, Lucas Stach,
Leonard Crestez, A.s. Dong, Richard Zhu, linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
On Sun, Dec 16, 2018 at 03:09:13PM -0800, Andrey Smirnov wrote:
> Lorenzo:
>
> These are patches fixing things pointed out by Bjorn in [1] and [2] as
> well as something I noticed while giving pci/next branch a try (#endif
> placement). Hopefully this is at least somewhat helpful.
>
> Thanks,
> Andrey Smirnov
>
> [1] lkml.kernel.org/r/20181214203828.GD20725@google.com
> [2] lkml.kernel.org/r/20181214203042.GC20725@google.com
>
> Andrey Smirnov (3):
> PCI: imx6: Fixup Kconfig and #endif placement
> PCI: imx6: Invert checks in imx6_pcie_reset_phy() and
> imx6_setup_phy_mpll()
> PCI: imx6: Make fallthrough comments more consistent
>
> drivers/pci/controller/dwc/Kconfig | 4 ++--
> drivers/pci/controller/dwc/pci-imx6.c | 29 ++++++++++++++++++---------
> 2 files changed, 21 insertions(+), 12 deletions(-)
Andrey,
please follow-up Lucas' remarks and submit a self-consistent patchset
that I can apply on top of my pci/dwc branch (where I removed your
previously queued patches).
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent
2018-12-16 23:09 ` [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent Andrey Smirnov
@ 2018-12-17 13:58 ` Bjorn Helgaas
2018-12-17 18:03 ` Andrey Smirnov
0 siblings, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2018-12-17 13:58 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Lorenzo Pieralisi, Fabio Estevam, Chris Healy, Lucas Stach,
Leonard Crestez, A.s. Dong, Richard Zhu, linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
On Sun, Dec 16, 2018 at 03:09:16PM -0800, Andrey Smirnov wrote:
> Convert all fallthrough comments to say "fall through", as well as
> modify their placement to the point where the "break" would normally
> be.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Leonard Crestez <leonard.crestez@nxp.com>
> Cc: "A.s. Dong" <aisheng.dong@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: linux-imx@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
I didn't make it very clear, but my suggestion was really to remove
the annotation completely; see below.
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 59658577e81d..a0510e185d44 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -362,7 +362,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>
> switch (imx6_pcie->variant) {
> case IMX7D:
> - case IMX8MQ: /* FALLTHROUGH */
> + /* fall through */
> + case IMX8MQ:
> reset_control_assert(imx6_pcie->pciephy_reset);
> reset_control_assert(imx6_pcie->apps_reset);
> break;
IMO this use of "fall through" is superfluous and unusual in the Linux
source.
A "fall through" comment would be useful if the IMX7D case had
executable code but no "break". Then the comment shows that the
intent was to execute *both* the IMX7D code and the IMX8MQ code and
the lack of a "break" was intentional.
In this case, the intent is to treat IMX7D and IMX8MQ the same, and
there's no executable code specifically for the IMX7D. I think it's
easiest to read that when the list of identical cases is all together
without the comment in the middle, i.e., as
> case IMX7D:
> case IMX8MQ:
> reset_control_assert(imx6_pcie->pciephy_reset);
rather than this:
> case IMX7D:
> /* fall through */
> case IMX8MQ:
> reset_control_assert(imx6_pcie->pciephy_reset);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll()
2018-12-17 10:24 ` Leonard Crestez
@ 2018-12-17 17:11 ` Andrey Smirnov
0 siblings, 0 replies; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-17 17:11 UTC (permalink / raw)
To: Leonard Crestez
Cc: Lorenzo Pieralisi, Lucas Stach, Stefan Agner, Bjorn Helgaas,
Fabio Estevam, Chris Healy, Aisheng Dong, Richard Zhu,
dl-linux-imx, linux-arm-kernel, linux-kernel, linux-pci
On Mon, Dec 17, 2018 at 2:24 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On 12/17/2018 1:09 AM, Andrey Smirnov wrote:
> > In order to avoid having potentially ever growing list of variants
> > that don't support methods implemented in imx6_pcie_reset_phy() and
> > imx6_setup_phy_mpll(), change logical checks in the to check for SoC's
> > that _do_ support what they implement. While at it, share the code via
> > a small helper function.
> >
> > +static bool imx6_pcie_has_imx6_phy(struct imx6_pcie *imx6_pcie)
> > +{
> > + return imx6_pcie->variant == IMX6Q ||
> > + imx6_pcie->variant == IMX6SX ||
> > + imx6_pcie->variant == IMX6QP;
> > +}
>
> This would be an ideal match for adding a field inside drvdata, sadly
> that was part of a series which stalled:
>
> https://patchwork.kernel.org/patch/10712261/
OK, I can pick up that individual patch into this series.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ"
2018-12-17 11:00 ` [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Lorenzo Pieralisi
@ 2018-12-17 17:18 ` Andrey Smirnov
0 siblings, 0 replies; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-17 17:18 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Fabio Estevam, Chris Healy, Lucas Stach,
Leonard Crestez, A.s. Dong, Richard Zhu, dl-linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
On Mon, Dec 17, 2018 at 3:01 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Sun, Dec 16, 2018 at 03:09:13PM -0800, Andrey Smirnov wrote:
> > Lorenzo:
> >
> > These are patches fixing things pointed out by Bjorn in [1] and [2] as
> > well as something I noticed while giving pci/next branch a try (#endif
> > placement). Hopefully this is at least somewhat helpful.
> >
> > Thanks,
> > Andrey Smirnov
> >
> > [1] lkml.kernel.org/r/20181214203828.GD20725@google.com
> > [2] lkml.kernel.org/r/20181214203042.GC20725@google.com
> >
> > Andrey Smirnov (3):
> > PCI: imx6: Fixup Kconfig and #endif placement
> > PCI: imx6: Invert checks in imx6_pcie_reset_phy() and
> > imx6_setup_phy_mpll()
> > PCI: imx6: Make fallthrough comments more consistent
> >
> > drivers/pci/controller/dwc/Kconfig | 4 ++--
> > drivers/pci/controller/dwc/pci-imx6.c | 29 ++++++++++++++++++---------
> > 2 files changed, 21 insertions(+), 12 deletions(-)
>
> Andrey,
>
> please follow-up Lucas' remarks and submit a self-consistent patchset
> that I can apply on top of my pci/dwc branch (where I removed your
> previously queued patches).
>
Sure, will do.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent
2018-12-17 13:58 ` Bjorn Helgaas
@ 2018-12-17 18:03 ` Andrey Smirnov
0 siblings, 0 replies; 10+ messages in thread
From: Andrey Smirnov @ 2018-12-17 18:03 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Lorenzo Pieralisi, Fabio Estevam, Chris Healy, Lucas Stach,
Leonard Crestez, A.s. Dong, Richard Zhu, dl-linux-imx,
linux-arm-kernel, linux-kernel, linux-pci
On Mon, Dec 17, 2018 at 5:58 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Sun, Dec 16, 2018 at 03:09:16PM -0800, Andrey Smirnov wrote:
> > Convert all fallthrough comments to say "fall through", as well as
> > modify their placement to the point where the "break" would normally
> > be.
> >
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Leonard Crestez <leonard.crestez@nxp.com>
> > Cc: "A.s. Dong" <aisheng.dong@nxp.com>
> > Cc: Richard Zhu <hongxing.zhu@nxp.com>
> > Cc: linux-imx@nxp.com
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-pci@vger.kernel.org
> > Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
>
> I didn't make it very clear, but my suggestion was really to remove
> the annotation completely; see below.
>
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 59658577e81d..a0510e185d44 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -362,7 +362,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >
> > switch (imx6_pcie->variant) {
> > case IMX7D:
> > - case IMX8MQ: /* FALLTHROUGH */
> > + /* fall through */
> > + case IMX8MQ:
> > reset_control_assert(imx6_pcie->pciephy_reset);
> > reset_control_assert(imx6_pcie->apps_reset);
> > break;
>
> IMO this use of "fall through" is superfluous and unusual in the Linux
> source.
>
> A "fall through" comment would be useful if the IMX7D case had
> executable code but no "break". Then the comment shows that the
> intent was to execute *both* the IMX7D code and the IMX8MQ code and
> the lack of a "break" was intentional.
>
> In this case, the intent is to treat IMX7D and IMX8MQ the same, and
> there's no executable code specifically for the IMX7D. I think it's
> easiest to read that when the list of identical cases is all together
> without the comment in the middle, i.e., as
>
> > case IMX7D:
> > case IMX8MQ:
> > reset_control_assert(imx6_pcie->pciephy_reset);
>
> rather than this:
>
> > case IMX7D:
> > /* fall through */
> > case IMX8MQ:
> > reset_control_assert(imx6_pcie->pciephy_reset);
OK, understood, will remove in next version.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-17 18:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-16 23:09 [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Andrey Smirnov
2018-12-16 23:09 ` [PATCH 1/3] PCI: imx6: Fixup Kconfig and #endif placement Andrey Smirnov
2018-12-16 23:09 ` [PATCH 2/3] PCI: imx6: Invert checks in imx6_pcie_reset_phy() and imx6_setup_phy_mpll() Andrey Smirnov
2018-12-17 10:24 ` Leonard Crestez
2018-12-17 17:11 ` Andrey Smirnov
2018-12-16 23:09 ` [PATCH 3/3] PCI: imx6: Make fallthrough comments more consistent Andrey Smirnov
2018-12-17 13:58 ` Bjorn Helgaas
2018-12-17 18:03 ` Andrey Smirnov
2018-12-17 11:00 ` [PATCH 0/3] Fixes for "PCIE support for i.MX8MQ" Lorenzo Pieralisi
2018-12-17 17:18 ` Andrey Smirnov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).