| Message ID | 20240410004832.3726922-5-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | PCI: rcar-gen4: Add R-Car V4H support | expand |
On Wed, Apr 10, 2024 at 09:48:29AM +0900, Yoshihiro Shimoda wrote: > This driver supports r8a779f0 now. In the future, add support for > r8a779g0 and r8a779h0. To support these new SoCs, need other > initializing settings. So, at first, add rcar_gen4_pcie_platdata > and have a member with mode. No behavior changes. > How about, "In order to support future SoCs such as r8a779g0 and r8a779h0 that require different initialization settings, let's introduce SoC specific driver data with the initial member being the device mode. No functional change." > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 30 ++++++++++++++------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > index 0be760ed420b..da2821d6efce 100644 > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > @@ -48,11 +48,15 @@ > #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000 > #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800 > > +struct rcar_gen4_pcie_platdata { Common naming convention is 'drvdata'. > + enum dw_pcie_device_mode mode; > +}; > + > struct rcar_gen4_pcie { > struct dw_pcie dw; > void __iomem *base; > struct platform_device *pdev; > - enum dw_pcie_device_mode mode; > + const struct rcar_gen4_pcie_platdata *platdata; > }; > #define to_rcar_gen4_pcie(_dw) container_of(_dw, struct rcar_gen4_pcie, dw) > > @@ -137,7 +141,7 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw) > * Since dw_pcie_setup_rc() sets it once, PCIe Gen2 will be trained. > * So, this needs remaining times for up to PCIe Gen4 if RC mode. > */ > - if (changes && rcar->mode == DW_PCIE_RC_TYPE) > + if (changes && rcar->platdata->mode == DW_PCIE_RC_TYPE) I'd recommend checking for the existence of the drvdata first. But if you are sure that it will be present for all SoCs, then it can be left. - Mani > changes--; > > for (i = 0; i < changes; i++) { > @@ -172,9 +176,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) > reset_control_assert(dw->core_rsts[DW_PCIE_PWR_RST].rstc); > > val = readl(rcar->base + PCIEMSR0); > - if (rcar->mode == DW_PCIE_RC_TYPE) { > + if (rcar->platdata->mode == DW_PCIE_RC_TYPE) { > val |= DEVICE_TYPE_RC; > - } else if (rcar->mode == DW_PCIE_EP_TYPE) { > + } else if (rcar->platdata->mode == DW_PCIE_EP_TYPE) { > val |= DEVICE_TYPE_EP; > } else { > ret = -EINVAL; > @@ -437,9 +441,9 @@ static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar) > /* Common */ > static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar) > { > - rcar->mode = (uintptr_t)of_device_get_match_data(&rcar->pdev->dev); > + rcar->platdata = of_device_get_match_data(&rcar->pdev->dev); > > - switch (rcar->mode) { > + switch (rcar->platdata->mode) { > case DW_PCIE_RC_TYPE: > return rcar_gen4_add_dw_pcie_rp(rcar); > case DW_PCIE_EP_TYPE: > @@ -480,7 +484,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev) > > static void rcar_gen4_remove_dw_pcie(struct rcar_gen4_pcie *rcar) > { > - switch (rcar->mode) { > + switch (rcar->platdata->mode) { > case DW_PCIE_RC_TYPE: > rcar_gen4_remove_dw_pcie_rp(rcar); > break; > @@ -500,14 +504,22 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev) > rcar_gen4_pcie_unprepare(rcar); > } > > +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie = { > + .mode = DW_PCIE_RC_TYPE, > +}; > + > +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie_ep = { > + .mode = DW_PCIE_EP_TYPE, > +}; > + > static const struct of_device_id rcar_gen4_pcie_of_match[] = { > { > .compatible = "renesas,rcar-gen4-pcie", > - .data = (void *)DW_PCIE_RC_TYPE, > + .data = &platdata_rcar_gen4_pcie, > }, > { > .compatible = "renesas,rcar-gen4-pcie-ep", > - .data = (void *)DW_PCIE_EP_TYPE, > + .data = &platdata_rcar_gen4_pcie_ep, > }, > {}, > }; > -- > 2.25.1 > >
Hello Manivannan, > From: Manivannan Sadhasivam, Sent: Thursday, April 11, 2024 2:17 AM > > On Wed, Apr 10, 2024 at 09:48:29AM +0900, Yoshihiro Shimoda wrote: > > This driver supports r8a779f0 now. In the future, add support for > > r8a779g0 and r8a779h0. To support these new SoCs, need other > > initializing settings. So, at first, add rcar_gen4_pcie_platdata > > and have a member with mode. No behavior changes. > > > > How about, > > "In order to support future SoCs such as r8a779g0 and r8a779h0 that require > different initialization settings, let's introduce SoC specific driver data with > the initial member being the device mode. No functional change." Thank you for the suggestion. I'll replace the description. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/pci/controller/dwc/pcie-rcar-gen4.c | 30 ++++++++++++++------- > > 1 file changed, 21 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > > index 0be760ed420b..da2821d6efce 100644 > > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c > > @@ -48,11 +48,15 @@ > > #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000 > > #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800 > > > > +struct rcar_gen4_pcie_platdata { > > Common naming convention is 'drvdata'. I got it. I'll rename it. > > + enum dw_pcie_device_mode mode; > > +}; > > + > > struct rcar_gen4_pcie { > > struct dw_pcie dw; > > void __iomem *base; > > struct platform_device *pdev; > > - enum dw_pcie_device_mode mode; > > + const struct rcar_gen4_pcie_platdata *platdata; > > }; > > #define to_rcar_gen4_pcie(_dw) container_of(_dw, struct rcar_gen4_pcie, dw) > > > > @@ -137,7 +141,7 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw) > > * Since dw_pcie_setup_rc() sets it once, PCIe Gen2 will be trained. > > * So, this needs remaining times for up to PCIe Gen4 if RC mode. > > */ > > - if (changes && rcar->mode == DW_PCIE_RC_TYPE) > > + if (changes && rcar->platdata->mode == DW_PCIE_RC_TYPE) > > I'd recommend checking for the existence of the drvdata first. But if you are > sure that it will be present for all SoCs, then it can be left. Since it will be present for all SoC, I will not change the code. Best regards, Yoshihiro Shimoda > - Mani > > > changes--; > > > > for (i = 0; i < changes; i++) { > > @@ -172,9 +176,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) > > reset_control_assert(dw->core_rsts[DW_PCIE_PWR_RST].rstc); > > > > val = readl(rcar->base + PCIEMSR0); > > - if (rcar->mode == DW_PCIE_RC_TYPE) { > > + if (rcar->platdata->mode == DW_PCIE_RC_TYPE) { > > val |= DEVICE_TYPE_RC; > > - } else if (rcar->mode == DW_PCIE_EP_TYPE) { > > + } else if (rcar->platdata->mode == DW_PCIE_EP_TYPE) { > > val |= DEVICE_TYPE_EP; > > } else { > > ret = -EINVAL; > > @@ -437,9 +441,9 @@ static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar) > > /* Common */ > > static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar) > > { > > - rcar->mode = (uintptr_t)of_device_get_match_data(&rcar->pdev->dev); > > + rcar->platdata = of_device_get_match_data(&rcar->pdev->dev); > > > > - switch (rcar->mode) { > > + switch (rcar->platdata->mode) { > > case DW_PCIE_RC_TYPE: > > return rcar_gen4_add_dw_pcie_rp(rcar); > > case DW_PCIE_EP_TYPE: > > @@ -480,7 +484,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev) > > > > static void rcar_gen4_remove_dw_pcie(struct rcar_gen4_pcie *rcar) > > { > > - switch (rcar->mode) { > > + switch (rcar->platdata->mode) { > > case DW_PCIE_RC_TYPE: > > rcar_gen4_remove_dw_pcie_rp(rcar); > > break; > > @@ -500,14 +504,22 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev) > > rcar_gen4_pcie_unprepare(rcar); > > } > > > > +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie = { > > + .mode = DW_PCIE_RC_TYPE, > > +}; > > + > > +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie_ep = { > > + .mode = DW_PCIE_EP_TYPE, > > +}; > > + > > static const struct of_device_id rcar_gen4_pcie_of_match[] = { > > { > > .compatible = "renesas,rcar-gen4-pcie", > > - .data = (void *)DW_PCIE_RC_TYPE, > > + .data = &platdata_rcar_gen4_pcie, > > }, > > { > > .compatible = "renesas,rcar-gen4-pcie-ep", > > - .data = (void *)DW_PCIE_EP_TYPE, > > + .data = &platdata_rcar_gen4_pcie_ep, > > }, > > {}, > > }; > > -- > > 2.25.1 > > > > > > -- > மணிவண்ணன் சதாசிவம்
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c index 0be760ed420b..da2821d6efce 100644 --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c @@ -48,11 +48,15 @@ #define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET 0x1000 #define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET 0x800 +struct rcar_gen4_pcie_platdata { + enum dw_pcie_device_mode mode; +}; + struct rcar_gen4_pcie { struct dw_pcie dw; void __iomem *base; struct platform_device *pdev; - enum dw_pcie_device_mode mode; + const struct rcar_gen4_pcie_platdata *platdata; }; #define to_rcar_gen4_pcie(_dw) container_of(_dw, struct rcar_gen4_pcie, dw) @@ -137,7 +141,7 @@ static int rcar_gen4_pcie_start_link(struct dw_pcie *dw) * Since dw_pcie_setup_rc() sets it once, PCIe Gen2 will be trained. * So, this needs remaining times for up to PCIe Gen4 if RC mode. */ - if (changes && rcar->mode == DW_PCIE_RC_TYPE) + if (changes && rcar->platdata->mode == DW_PCIE_RC_TYPE) changes--; for (i = 0; i < changes; i++) { @@ -172,9 +176,9 @@ static int rcar_gen4_pcie_common_init(struct rcar_gen4_pcie *rcar) reset_control_assert(dw->core_rsts[DW_PCIE_PWR_RST].rstc); val = readl(rcar->base + PCIEMSR0); - if (rcar->mode == DW_PCIE_RC_TYPE) { + if (rcar->platdata->mode == DW_PCIE_RC_TYPE) { val |= DEVICE_TYPE_RC; - } else if (rcar->mode == DW_PCIE_EP_TYPE) { + } else if (rcar->platdata->mode == DW_PCIE_EP_TYPE) { val |= DEVICE_TYPE_EP; } else { ret = -EINVAL; @@ -437,9 +441,9 @@ static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar) /* Common */ static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar) { - rcar->mode = (uintptr_t)of_device_get_match_data(&rcar->pdev->dev); + rcar->platdata = of_device_get_match_data(&rcar->pdev->dev); - switch (rcar->mode) { + switch (rcar->platdata->mode) { case DW_PCIE_RC_TYPE: return rcar_gen4_add_dw_pcie_rp(rcar); case DW_PCIE_EP_TYPE: @@ -480,7 +484,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev) static void rcar_gen4_remove_dw_pcie(struct rcar_gen4_pcie *rcar) { - switch (rcar->mode) { + switch (rcar->platdata->mode) { case DW_PCIE_RC_TYPE: rcar_gen4_remove_dw_pcie_rp(rcar); break; @@ -500,14 +504,22 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev) rcar_gen4_pcie_unprepare(rcar); } +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie = { + .mode = DW_PCIE_RC_TYPE, +}; + +static struct rcar_gen4_pcie_platdata platdata_rcar_gen4_pcie_ep = { + .mode = DW_PCIE_EP_TYPE, +}; + static const struct of_device_id rcar_gen4_pcie_of_match[] = { { .compatible = "renesas,rcar-gen4-pcie", - .data = (void *)DW_PCIE_RC_TYPE, + .data = &platdata_rcar_gen4_pcie, }, { .compatible = "renesas,rcar-gen4-pcie-ep", - .data = (void *)DW_PCIE_EP_TYPE, + .data = &platdata_rcar_gen4_pcie_ep, }, {}, };
This driver supports r8a779f0 now. In the future, add support for r8a779g0 and r8a779h0. To support these new SoCs, need other initializing settings. So, at first, add rcar_gen4_pcie_platdata and have a member with mode. No behavior changes. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/controller/dwc/pcie-rcar-gen4.c | 30 ++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-)