| Message ID | 20230426045557.3613826-8-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: rcar-gen4: Add R-Car Gen4 PCIe support | expand |
On Wed, Apr 26, 2023 at 01:55:43PM +0900, Yoshihiro Shimoda wrote: > Add "code" and "routing" into struct dw_pcie_outbound_atu for > sending MSG by iATU in the PCIe endpoint mode in near the future. [PATCH v14 07/21] PCI: dwc: Add members into struct dw_pcie_outbound_atu what about the next subject: "PCI: dwc: Add "code" and "routing" outbound iATU settings" or a more generic version: "PCI: dwc: Add outbound MSG TLPs support" ? It would have been also nice to explain in the patch log (or/and in the code) why the "PCIE_ATU_INHIBIT_PAYLOAD" and "PCIE_ATU_HEADER_SUB_ENABLE" flags are required to be set. Other than that the change looks good. -Serge(y) > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/controller/dwc/pcie-designware.c | 7 +++++-- > drivers/pci/controller/dwc/pcie-designware.h | 5 +++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 782c4b34d0a3..e8d4d5bde2d3 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -497,7 +497,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, > dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_TARGET, > upper_32_bits(atu->pci_addr)); > > - val = atu->type | PCIE_ATU_FUNC_NUM(atu->func_no); > + val = atu->type | atu->routing | PCIE_ATU_FUNC_NUM(atu->func_no); > if (upper_32_bits(limit_addr) > upper_32_bits(atu->cpu_addr) && > dw_pcie_ver_is_ge(pci, 460A)) > val |= PCIE_ATU_INCREASE_REGION_SIZE; > @@ -505,7 +505,10 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, > val = dw_pcie_enable_ecrc(val); > dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val); > > - dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE); > + val = PCIE_ATU_ENABLE; > + if (atu->type == PCIE_ATU_TYPE_MSG) > + val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | atu->code; > + dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, val); > > /* > * Make sure ATU enable takes effect before any subsequent config > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 81c7558a4718..954d504890a1 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -150,11 +150,14 @@ > #define PCIE_ATU_TYPE_IO 0x2 > #define PCIE_ATU_TYPE_CFG0 0x4 > #define PCIE_ATU_TYPE_CFG1 0x5 > +#define PCIE_ATU_TYPE_MSG 0x10 > #define PCIE_ATU_TD BIT(8) > #define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20) > #define PCIE_ATU_REGION_CTRL2 0x004 > #define PCIE_ATU_ENABLE BIT(31) > #define PCIE_ATU_BAR_MODE_ENABLE BIT(30) > +#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22) > +#define PCIE_ATU_HEADER_SUB_ENABLE BIT(21) > #define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19) > #define PCIE_ATU_LOWER_BASE 0x008 > #define PCIE_ATU_UPPER_BASE 0x00C > @@ -298,6 +301,8 @@ struct dw_pcie_outbound_atu { > int index; > int type; > u8 func_no; > + u8 code; > + u8 routing; > }; > > struct dw_pcie_host_ops { > -- > 2.25.1 >
Hi Serge, > From: Serge Semin, Sent: Tuesday, May 2, 2023 4:15 AM > > On Wed, Apr 26, 2023 at 01:55:43PM +0900, Yoshihiro Shimoda wrote: > > Add "code" and "routing" into struct dw_pcie_outbound_atu for > > sending MSG by iATU in the PCIe endpoint mode in near the future. > > [PATCH v14 07/21] PCI: dwc: Add members into struct dw_pcie_outbound_atu > > what about the next subject: > "PCI: dwc: Add "code" and "routing" outbound iATU settings" > or a more generic version: > "PCI: dwc: Add outbound MSG TLPs support" > ? Thank you for the suggestion. The generic version looks good to me. > It would have been also nice to explain in the patch log (or/and in > the code) why the "PCIE_ATU_INHIBIT_PAYLOAD" and > "PCIE_ATU_HEADER_SUB_ENABLE" flags are required to be set. I got it. I'll add explanation like below: PCIE_ATU_INHIBIT_PAYLOAD is set for setting TLP type Msg instead of MsgD. PCIE_ATU_HEADER_SUB_ENABLE is set for issuing the translated TLP header by using PCIE_ATU_{LOW,UPP}ER_TARGET registers' values. > Other than that the change looks good. Thanks! Best regards, Yoshihiro Shimoda > -Serge(y) > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/pci/controller/dwc/pcie-designware.c | 7 +++++-- > > drivers/pci/controller/dwc/pcie-designware.h | 5 +++++ > > 2 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > index 782c4b34d0a3..e8d4d5bde2d3 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > @@ -497,7 +497,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, > > dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_TARGET, > > upper_32_bits(atu->pci_addr)); > > > > - val = atu->type | PCIE_ATU_FUNC_NUM(atu->func_no); > > + val = atu->type | atu->routing | PCIE_ATU_FUNC_NUM(atu->func_no); > > if (upper_32_bits(limit_addr) > upper_32_bits(atu->cpu_addr) && > > dw_pcie_ver_is_ge(pci, 460A)) > > val |= PCIE_ATU_INCREASE_REGION_SIZE; > > @@ -505,7 +505,10 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, > > val = dw_pcie_enable_ecrc(val); > > dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val); > > > > - dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE); > > + val = PCIE_ATU_ENABLE; > > + if (atu->type == PCIE_ATU_TYPE_MSG) > > + val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | atu->code; > > + dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, val); > > > > /* > > * Make sure ATU enable takes effect before any subsequent config > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > > index 81c7558a4718..954d504890a1 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -150,11 +150,14 @@ > > #define PCIE_ATU_TYPE_IO 0x2 > > #define PCIE_ATU_TYPE_CFG0 0x4 > > #define PCIE_ATU_TYPE_CFG1 0x5 > > +#define PCIE_ATU_TYPE_MSG 0x10 > > #define PCIE_ATU_TD BIT(8) > > #define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20) > > #define PCIE_ATU_REGION_CTRL2 0x004 > > #define PCIE_ATU_ENABLE BIT(31) > > #define PCIE_ATU_BAR_MODE_ENABLE BIT(30) > > +#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22) > > +#define PCIE_ATU_HEADER_SUB_ENABLE BIT(21) > > #define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19) > > #define PCIE_ATU_LOWER_BASE 0x008 > > #define PCIE_ATU_UPPER_BASE 0x00C > > @@ -298,6 +301,8 @@ struct dw_pcie_outbound_atu { > > int index; > > int type; > > u8 func_no; > > + u8 code; > > + u8 routing; > > }; > > > > struct dw_pcie_host_ops { > > -- > > 2.25.1 > >
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 782c4b34d0a3..e8d4d5bde2d3 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -497,7 +497,7 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_TARGET, upper_32_bits(atu->pci_addr)); - val = atu->type | PCIE_ATU_FUNC_NUM(atu->func_no); + val = atu->type | atu->routing | PCIE_ATU_FUNC_NUM(atu->func_no); if (upper_32_bits(limit_addr) > upper_32_bits(atu->cpu_addr) && dw_pcie_ver_is_ge(pci, 460A)) val |= PCIE_ATU_INCREASE_REGION_SIZE; @@ -505,7 +505,10 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, val = dw_pcie_enable_ecrc(val); dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val); - dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE); + val = PCIE_ATU_ENABLE; + if (atu->type == PCIE_ATU_TYPE_MSG) + val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | atu->code; + dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, val); /* * Make sure ATU enable takes effect before any subsequent config diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 81c7558a4718..954d504890a1 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -150,11 +150,14 @@ #define PCIE_ATU_TYPE_IO 0x2 #define PCIE_ATU_TYPE_CFG0 0x4 #define PCIE_ATU_TYPE_CFG1 0x5 +#define PCIE_ATU_TYPE_MSG 0x10 #define PCIE_ATU_TD BIT(8) #define PCIE_ATU_FUNC_NUM(pf) ((pf) << 20) #define PCIE_ATU_REGION_CTRL2 0x004 #define PCIE_ATU_ENABLE BIT(31) #define PCIE_ATU_BAR_MODE_ENABLE BIT(30) +#define PCIE_ATU_INHIBIT_PAYLOAD BIT(22) +#define PCIE_ATU_HEADER_SUB_ENABLE BIT(21) #define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19) #define PCIE_ATU_LOWER_BASE 0x008 #define PCIE_ATU_UPPER_BASE 0x00C @@ -298,6 +301,8 @@ struct dw_pcie_outbound_atu { int index; int type; u8 func_no; + u8 code; + u8 routing; }; struct dw_pcie_host_ops {
Add "code" and "routing" into struct dw_pcie_outbound_atu for sending MSG by iATU in the PCIe endpoint mode in near the future. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/controller/dwc/pcie-designware.c | 7 +++++-- drivers/pci/controller/dwc/pcie-designware.h | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-)