[PATCH v3 0/2] PCI: rcar: Fix missing MACCTLR register setting (take2)

[PATCH v3 0/2] PCI: rcar: Fix missing MACCTLR register setting (take2)

[Yoshihiro Shimoda]

This patch series is based on the latest pci/rcar branch of Lorenzo's pci.git.
The commit 175cc093888e ("PCI: rcar: Fix missing MACCTLR register setting
in rcar_pcie_hw_init()") description/code don't follow the manual
accurately, so that it's difficult to understand. So, this patch
series reverts the commit at first, and then applies a new fixed patch.

Reference:
https://marc.info/?l=linux-renesas-soc&m=157242422327368&w=2

Changes from v2:
 - Rebase on the latest pcir/rcar branch.
 - Add Euguniu-san's Reported-by in the patch 2/2.
 - Add the bits definitions instead of magic number to make the initial value
   so that I don't add Euguniu-san's Reviewed-by tag in the patch 2/2.
 https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=196557

Changes from v1:
 - Follow -stable rule in patch 1/2.
 - Add some comments about SPCHG bit of MACCTLR register.
https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=195717

Yoshihiro Shimoda (2):
  Revert "PCI: rcar: Fix missing MACCTLR register setting in
    rcar_pcie_hw_init()"
  PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

 drivers/pci/controller/pcie-rcar.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH v3 1/2] Revert "PCI: rcar: Fix missing MACCTLR register setting in rcar_pcie_hw_init()"

[Yoshihiro Shimoda]

This reverts commit 175cc093888ee74a17c4dd5f99ba9a6bc86de5be.

The commit description/code don't follow the manual accurately,
it's difficult to understand. So, this patch reverts the commit.

Fixes: 175cc093888e ("PCI: rcar: Fix missing MACCTLR register setting in rcar_pcie_hw_init()"
Cc: <stable@vger.kernel.org>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/pcie-rcar.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index 0dadccb..40d8c54 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -91,7 +91,6 @@
 #define  LINK_SPEED_2_5GTS	(1 << 16)
 #define  LINK_SPEED_5_0GTS	(2 << 16)
 #define MACCTLR			0x011058
-#define  MACCTLR_RESERVED	BIT(0)
 #define  SPEED_CHANGE		BIT(24)
 #define  SCRAMBLE_DISABLE	BIT(27)
 #define PMSR			0x01105c
@@ -614,8 +613,6 @@ static int rcar_pcie_hw_init(struct rcar_pcie *pcie)
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		rcar_pci_write_reg(pcie, 0x801f0000, PCIEMSITXR);
 
-	rcar_rmw32(pcie, MACCTLR, MACCTLR_RESERVED, 0);
-
 	/* Finish initialization - establish a PCI Express link */
 	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
 
@@ -1238,7 +1235,6 @@ static int rcar_pcie_resume_noirq(struct device *dev)
 		return 0;
 
 	/* Re-establish the PCIe link */
-	rcar_rmw32(pcie, MACCTLR, MACCTLR_RESERVED, 0);
 	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
 	return rcar_pcie_wait_for_dl(pcie);
 }
-- 
2.7.4

Re: [PATCH v3 1/2] Revert "PCI: rcar: Fix missing MACCTLR register setting in rcar_pcie_hw_init()"

[Geert Uytterhoeven]

On Tue, Nov 5, 2019 at 3:48 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This reverts commit 175cc093888ee74a17c4dd5f99ba9a6bc86de5be.
>
> The commit description/code don't follow the manual accurately,
> it's difficult to understand. So, this patch reverts the commit.
>
> Fixes: 175cc093888e ("PCI: rcar: Fix missing MACCTLR register setting in rcar_pcie_hw_init()"
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Yoshihiro Shimoda]

According to the R-Car Gen2/3 manual, "Be sure to write the initial
value (= H'80FF 0000) to MACCTLR before enabling PCIETCTLR.CFINIT".
To avoid unexpected behaviors, this patch fixes it. Note that
the SPCHG bit of MACCTLR register description said "Only writing 1
is valid and writing 0 is invalid" but this "invalid" means
"ignored", not "prohibited". So, any documentation conflict doesn't
exist about writing the MACCTLR register.

Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
Cc: <stable@vger.kernel.org> # v5.2+
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/pcie-rcar.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index 40d8c54..1bfec1f 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -91,8 +91,12 @@
 #define  LINK_SPEED_2_5GTS	(1 << 16)
 #define  LINK_SPEED_5_0GTS	(2 << 16)
 #define MACCTLR			0x011058
+#define  MACCTLR_RESERVED23_16	GENMASK(23, 16)
 #define  SPEED_CHANGE		BIT(24)
 #define  SCRAMBLE_DISABLE	BIT(27)
+#define  LTSMDIS		BIT(31)
+	 /* Be sure to write the initial value (H'80FF 0000) to MACCTLR */
+#define  MACCTLR_INIT_VAL	(LTSMDIS | MACCTLR_RESERVED23_16)
 #define PMSR			0x01105c
 #define MACS2R			0x011078
 #define MACCGSPSETR		0x011084
@@ -613,6 +617,8 @@ static int rcar_pcie_hw_init(struct rcar_pcie *pcie)
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		rcar_pci_write_reg(pcie, 0x801f0000, PCIEMSITXR);
 
+	rcar_pci_write_reg(pcie, MACCTLR_INIT_VAL, MACCTLR);
+
 	/* Finish initialization - establish a PCI Express link */
 	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
 
@@ -1235,6 +1241,7 @@ static int rcar_pcie_resume_noirq(struct device *dev)
 		return 0;
 
 	/* Re-establish the PCIe link */
+	rcar_pci_write_reg(pcie, MACCTLR_INIT_VAL, MACCTLR);
 	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
 	return rcar_pcie_wait_for_dl(pcie);
 }
-- 
2.7.4

Re: [PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Nov 5, 2019 at 3:48 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> According to the R-Car Gen2/3 manual, "Be sure to write the initial
> value (= H'80FF 0000) to MACCTLR before enabling PCIETCTLR.CFINIT".
> To avoid unexpected behaviors, this patch fixes it. Note that
> the SPCHG bit of MACCTLR register description said "Only writing 1
> is valid and writing 0 is invalid" but this "invalid" means
> "ignored", not "prohibited". So, any documentation conflict doesn't
> exist about writing the MACCTLR register.
>
> Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
> Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
> Cc: <stable@vger.kernel.org> # v5.2+
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- a/drivers/pci/controller/pcie-rcar.c
> +++ b/drivers/pci/controller/pcie-rcar.c
> @@ -91,8 +91,12 @@
>  #define  LINK_SPEED_2_5GTS     (1 << 16)
>  #define  LINK_SPEED_5_0GTS     (2 << 16)
>  #define MACCTLR                        0x011058
> +#define  MACCTLR_RESERVED23_16 GENMASK(23, 16)

MACCTLR_NFTS_MASK?

>  #define  SPEED_CHANGE          BIT(24)
>  #define  SCRAMBLE_DISABLE      BIT(27)
> +#define  LTSMDIS               BIT(31)
> +        /* Be sure to write the initial value (H'80FF 0000) to MACCTLR */

Do we need this comment?

> +#define  MACCTLR_INIT_VAL      (LTSMDIS | MACCTLR_RESERVED23_16)
>  #define PMSR                   0x01105c
>  #define MACS2R                 0x011078
>  #define MACCGSPSETR            0x011084

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for your review!

> From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 5:50 PM
> 
> Hi Shimoda-san,
> 
> On Tue, Nov 5, 2019 at 3:48 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > According to the R-Car Gen2/3 manual, "Be sure to write the initial
> > value (= H'80FF 0000) to MACCTLR before enabling PCIETCTLR.CFINIT".
> > To avoid unexpected behaviors, this patch fixes it. Note that
> > the SPCHG bit of MACCTLR register description said "Only writing 1
> > is valid and writing 0 is invalid" but this "invalid" means
> > "ignored", not "prohibited". So, any documentation conflict doesn't
> > exist about writing the MACCTLR register.
> >
> > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
> > Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
> > Cc: <stable@vger.kernel.org> # v5.2+
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/pci/controller/pcie-rcar.c
> > +++ b/drivers/pci/controller/pcie-rcar.c
> > @@ -91,8 +91,12 @@
> >  #define  LINK_SPEED_2_5GTS     (1 << 16)
> >  #define  LINK_SPEED_5_0GTS     (2 << 16)
> >  #define MACCTLR                        0x011058
> > +#define  MACCTLR_RESERVED23_16 GENMASK(23, 16)
> 
> MACCTLR_NFTS_MASK?

I should have said on previous email thread [1] though,
since SH7786 PCIE HW manual said NFTS (NFTS) but
any R-Car SoCs' HW manual said just Reserved with H'FF,
so that I prefer to describe RESERVED instead of NFTS.
Do you agree?

[1]
https://marc.info/?l=linux-renesas-soc&m=157242422327368&w=2

> >  #define  SPEED_CHANGE          BIT(24)
> >  #define  SCRAMBLE_DISABLE      BIT(27)
> > +#define  LTSMDIS               BIT(31)
> > +        /* Be sure to write the initial value (H'80FF 0000) to MACCTLR */
> 
> Do we need this comment?

It's a little redundant. So, I'll remove it.

> > +#define  MACCTLR_INIT_VAL      (LTSMDIS | MACCTLR_RESERVED23_16)
> >  #define PMSR                   0x01105c
> >  #define MACS2R                 0x011078
> >  #define MACCGSPSETR            0x011084
> 
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

Best regards,
Yoshihiro Shimoda

Re: [PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Nov 5, 2019 at 10:26 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 5:50 PM
> > On Tue, Nov 5, 2019 at 3:48 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > According to the R-Car Gen2/3 manual, "Be sure to write the initial
> > > value (= H'80FF 0000) to MACCTLR before enabling PCIETCTLR.CFINIT".
> > > To avoid unexpected behaviors, this patch fixes it. Note that
> > > the SPCHG bit of MACCTLR register description said "Only writing 1
> > > is valid and writing 0 is invalid" but this "invalid" means
> > > "ignored", not "prohibited". So, any documentation conflict doesn't
> > > exist about writing the MACCTLR register.
> > >
> > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
> > > Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
> > > Cc: <stable@vger.kernel.org> # v5.2+
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/pci/controller/pcie-rcar.c
> > > +++ b/drivers/pci/controller/pcie-rcar.c
> > > @@ -91,8 +91,12 @@
> > >  #define  LINK_SPEED_2_5GTS     (1 << 16)
> > >  #define  LINK_SPEED_5_0GTS     (2 << 16)
> > >  #define MACCTLR                        0x011058
> > > +#define  MACCTLR_RESERVED23_16 GENMASK(23, 16)
> >
> > MACCTLR_NFTS_MASK?
>
> I should have said on previous email thread [1] though,
> since SH7786 PCIE HW manual said NFTS (NFTS) but
> any R-Car SoCs' HW manual said just Reserved with H'FF,
> so that I prefer to describe RESERVED instead of NFTS.
> Do you agree?
>
> [1]
> https://marc.info/?l=linux-renesas-soc&m=157242422327368&w=2

My personal stance is to make it as easy as possible for the reader of
the code ("optimize for reading, not for writing"), as code is written once,
but read many more times later.
This is not the first time register bits were documented before, and changed
to reserved later.
In this case the resemblance to the SH7786 PCIe block is obvious, and
the SH7786 hardware user's manual is available publicly.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 6:53 PM
> 
> Hi Shimoda-san,
> 
> On Tue, Nov 5, 2019 at 10:26 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 5:50 PM
> > > On Tue, Nov 5, 2019 at 3:48 AM Yoshihiro Shimoda
> > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > According to the R-Car Gen2/3 manual, "Be sure to write the initial
> > > > value (= H'80FF 0000) to MACCTLR before enabling PCIETCTLR.CFINIT".
> > > > To avoid unexpected behaviors, this patch fixes it. Note that
> > > > the SPCHG bit of MACCTLR register description said "Only writing 1
> > > > is valid and writing 0 is invalid" but this "invalid" means
> > > > "ignored", not "prohibited". So, any documentation conflict doesn't
> > > > exist about writing the MACCTLR register.
> > > >
> > > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
> > > > Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
> > > > Cc: <stable@vger.kernel.org> # v5.2+
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/pci/controller/pcie-rcar.c
> > > > +++ b/drivers/pci/controller/pcie-rcar.c
> > > > @@ -91,8 +91,12 @@
> > > >  #define  LINK_SPEED_2_5GTS     (1 << 16)
> > > >  #define  LINK_SPEED_5_0GTS     (2 << 16)
> > > >  #define MACCTLR                        0x011058
> > > > +#define  MACCTLR_RESERVED23_16 GENMASK(23, 16)
> > >
> > > MACCTLR_NFTS_MASK?
> >
> > I should have said on previous email thread [1] though,
> > since SH7786 PCIE HW manual said NFTS (NFTS) but
> > any R-Car SoCs' HW manual said just Reserved with H'FF,
> > so that I prefer to describe RESERVED instead of NFTS.
> > Do you agree?
> >
> > [1]
> > https://marc.info/?l=linux-renesas-soc&m=157242422327368&w=2
> 
> My personal stance is to make it as easy as possible for the reader of
> the code ("optimize for reading, not for writing"), as code is written once,
> but read many more times later.
> This is not the first time register bits were documented before, and changed
> to reserved later.
> In this case the resemblance to the SH7786 PCIe block is obvious, and
> the SH7786 hardware user's manual is available publicly.

Thank you for sharing your stance. I understood it. So, I'll fix it as following.
Is it acceptable?

#define  MACCTLR_NFTS_MASK	GENMASK(23, 16)	/* The name is from SH7786 */

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Nov 5, 2019 at 11:11 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 6:53 PM
> > On Tue, Nov 5, 2019 at 10:26 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 5:50 PM
> > > > On Tue, Nov 5, 2019 at 3:48 AM Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > > According to the R-Car Gen2/3 manual, "Be sure to write the initial
> > > > > value (= H'80FF 0000) to MACCTLR before enabling PCIETCTLR.CFINIT".
> > > > > To avoid unexpected behaviors, this patch fixes it. Note that
> > > > > the SPCHG bit of MACCTLR register description said "Only writing 1
> > > > > is valid and writing 0 is invalid" but this "invalid" means
> > > > > "ignored", not "prohibited". So, any documentation conflict doesn't
> > > > > exist about writing the MACCTLR register.
> > > > >
> > > > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > > > > Fixes: c25da4778803 ("PCI: rcar: Add Renesas R-Car PCIe driver")
> > > > > Fixes: be20bbcb0a8c ("PCI: rcar: Add the initialization of PCIe link in resume_noirq()")
> > > > > Cc: <stable@vger.kernel.org> # v5.2+
> > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- a/drivers/pci/controller/pcie-rcar.c
> > > > > +++ b/drivers/pci/controller/pcie-rcar.c
> > > > > @@ -91,8 +91,12 @@
> > > > >  #define  LINK_SPEED_2_5GTS     (1 << 16)
> > > > >  #define  LINK_SPEED_5_0GTS     (2 << 16)
> > > > >  #define MACCTLR                        0x011058
> > > > > +#define  MACCTLR_RESERVED23_16 GENMASK(23, 16)
> > > >
> > > > MACCTLR_NFTS_MASK?
> > >
> > > I should have said on previous email thread [1] though,
> > > since SH7786 PCIE HW manual said NFTS (NFTS) but
> > > any R-Car SoCs' HW manual said just Reserved with H'FF,
> > > so that I prefer to describe RESERVED instead of NFTS.
> > > Do you agree?
> > >
> > > [1]
> > > https://marc.info/?l=linux-renesas-soc&m=157242422327368&w=2
> >
> > My personal stance is to make it as easy as possible for the reader of
> > the code ("optimize for reading, not for writing"), as code is written once,
> > but read many more times later.
> > This is not the first time register bits were documented before, and changed
> > to reserved later.
> > In this case the resemblance to the SH7786 PCIe block is obvious, and
> > the SH7786 hardware user's manual is available publicly.
>
> Thank you for sharing your stance. I understood it. So, I'll fix it as following.
> Is it acceptable?
>
> #define  MACCTLR_NFTS_MASK      GENMASK(23, 16) /* The name is from SH7786 */

Sounds great to me.
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 2/2] PCI: rcar: Fix missing MACCTLR register setting in initialize sequence

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, November 5, 2019 7:26 PM
<snip>
> > > > > > --- a/drivers/pci/controller/pcie-rcar.c
> > > > > > +++ b/drivers/pci/controller/pcie-rcar.c
> > > > > > @@ -91,8 +91,12 @@
> > > > > >  #define  LINK_SPEED_2_5GTS     (1 << 16)
> > > > > >  #define  LINK_SPEED_5_0GTS     (2 << 16)
> > > > > >  #define MACCTLR                        0x011058
> > > > > > +#define  MACCTLR_RESERVED23_16 GENMASK(23, 16)
> > > > >
> > > > > MACCTLR_NFTS_MASK?
> > > >
> > > > I should have said on previous email thread [1] though,
> > > > since SH7786 PCIE HW manual said NFTS (NFTS) but
> > > > any R-Car SoCs' HW manual said just Reserved with H'FF,
> > > > so that I prefer to describe RESERVED instead of NFTS.
> > > > Do you agree?
> > > >
> > > > [1]
> > > > https://marc.info/?l=linux-renesas-soc&m=157242422327368&w=2
> > >
> > > My personal stance is to make it as easy as possible for the reader of
> > > the code ("optimize for reading, not for writing"), as code is written once,
> > > but read many more times later.
> > > This is not the first time register bits were documented before, and changed
> > > to reserved later.
> > > In this case the resemblance to the SH7786 PCIe block is obvious, and
> > > the SH7786 hardware user's manual is available publicly.
> >
> > Thank you for sharing your stance. I understood it. So, I'll fix it as following.
> > Is it acceptable?
> >
> > #define  MACCTLR_NFTS_MASK      GENMASK(23, 16) /* The name is from SH7786 */
> 
> Sounds great to me.
> Thanks!

Thank you for the reply! I got it!

Best regards,
Yoshihiro Shimoda