Re: [PATCH v16 07/22] PCI: dwc: Add outbound MSG TLPs support

[Serge Semin <fancer.lancer@gmail.com>]

On Wed, May 10, 2023 at 03:22:19PM +0900, Yoshihiro Shimoda wrote:
> Add "code" and "routing" into struct dw_pcie_outbound_atu for

structure name has been changed.

> sending MSG by iATU in the PCIe endpoint mode in near the future.
> PCIE_ATU_INHIBIT_PAYLOAD is set to issue TLP type of Msg instead of
> MsgD.

So your implementation implies the data-less messages only. This note
should have been added at least to the commit log. Ideally it would be
useful to have an in-situ comment above the code setting these flags.

> PCIE_ATU_HEADER_SUB_ENABLE is set to issue the translated
> TLP header by using PCIE_ATU_{LOW,UPP}ER_TARGET registers' values.

Why is that needed? Please elaborate in the patch log.

Other than that the change looks good.

* I'll get back to the series review tomorrow.

-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 bd85a73d354b..a7c724ba7385 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -498,7 +498,7 @@ 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(cpu_addr) &&
>  	    dw_pcie_ver_is_ge(pci, 460A))
>  		val |= PCIE_ATU_INCREASE_REGION_SIZE;
> @@ -506,7 +506,10 @@ 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 b8fa099bbed3..c83d1d176e62 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
> @@ -295,6 +298,8 @@ struct dw_pcie_ob_atu_cfg {
>  	int index;
>  	int type;
>  	u8 func_no;
> +	u8 code;
> +	u8 routing;
>  	u64 cpu_addr;
>  	u64 pci_addr;
>  	u64 size;
> -- 
> 2.25.1
>