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@xxxxxxxxxxx>
> > ---
> > 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
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
