| Message ID | 20241015090712.112674-1-dlemoal@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | [v2] PCI: endpoint: Improve pci_epc_ops::align_addr() interface | expand |
On Tue, Oct 15, 2024 at 06:07:12PM +0900, Damien Le Moal wrote: > The PCI endpoint controller operation interface for the align_addr() > operation uses the phys_addr_t type for the PCI address argument and > return a value using this type. This is not ideal as PCI addresses are > bus addresses, not regular memory physical addresses. Replace the use of > phys_addr_t for this operation with the generic u64 type. To be > consistent with this change the Designware driver implementation of this > operation (function dw_pcie_ep_align_addr()) as well as the type of PCI > address fields of struct pci_epc_map are also changed. > > Fixes: e98c99e2ccad ("PCI: endpoint: Introduce pci_epc_mem_map()/unmap()") > Fixes: cb6b7158fdf5 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation") > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > Changes from v1: > - Also updated the type of the pci_addr and map_pci_addr fields of > struct pci_epc_map. > > drivers/pci/controller/dwc/pcie-designware-ep.c | 6 +++--- > include/linux/pci-epc.h | 8 ++++---- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 0ada60487c25..df1460ed63d9 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -268,12 +268,12 @@ static int dw_pcie_find_index(struct dw_pcie_ep *ep, phys_addr_t addr, > return -EINVAL; > } > > -static phys_addr_t dw_pcie_ep_align_addr(struct pci_epc *epc, > - phys_addr_t pci_addr, size_t *pci_size, size_t *offset) > +static u64 dw_pcie_ep_align_addr(struct pci_epc *epc, u64 pci_addr, > + size_t *pci_size, size_t *offset) > { > struct dw_pcie_ep *ep = epc_get_drvdata(epc); > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > - phys_addr_t mask = pci->region_align - 1; > + u64 mask = pci->region_align - 1; > size_t ofst = pci_addr & mask; > > *pci_size = ALIGN(ofst + *pci_size, ep->page_size); > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index f4b8dc37e447..de8cc3658220 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -49,10 +49,10 @@ pci_epc_interface_string(enum pci_epc_interface_type type) > * @virt_addr: virtual address at which @pci_addr is mapped > */ > struct pci_epc_map { > - phys_addr_t pci_addr; > + u64 pci_addr; > size_t pci_size; > > - phys_addr_t map_pci_addr; > + u64 map_pci_addr; > size_t map_size; > > phys_addr_t phys_base; > @@ -93,8 +93,8 @@ struct pci_epc_ops { > struct pci_epf_bar *epf_bar); > void (*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > struct pci_epf_bar *epf_bar); > - phys_addr_t (*align_addr)(struct pci_epc *epc, phys_addr_t pci_addr, > - size_t *size, size_t *offset); > + u64 (*align_addr)(struct pci_epc *epc, u64 pci_addr, size_t *size, > + size_t *offset); > int (*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > phys_addr_t addr, u64 pci_addr, size_t size); > void (*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > -- > 2.47.0 > Reviewed-by: Niklas Cassel <cassel@kernel.org>
On Tue, Oct 15, 2024 at 06:07:12PM +0900, Damien Le Moal wrote: > The PCI endpoint controller operation interface for the align_addr() > operation uses the phys_addr_t type for the PCI address argument and > return a value using this type. This is not ideal as PCI addresses are > bus addresses, not regular memory physical addresses. Replace the use of > phys_addr_t for this operation with the generic u64 type. To be > consistent with this change the Designware driver implementation of this > operation (function dw_pcie_ep_align_addr()) as well as the type of PCI > address fields of struct pci_epc_map are also changed. > > Fixes: e98c99e2ccad ("PCI: endpoint: Introduce pci_epc_mem_map()/unmap()") > Fixes: cb6b7158fdf5 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation") > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > Changes from v1: > - Also updated the type of the pci_addr and map_pci_addr fields of > struct pci_epc_map. > > drivers/pci/controller/dwc/pcie-designware-ep.c | 6 +++--- > include/linux/pci-epc.h | 8 ++++---- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 0ada60487c25..df1460ed63d9 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -268,12 +268,12 @@ static int dw_pcie_find_index(struct dw_pcie_ep *ep, phys_addr_t addr, > return -EINVAL; > } > > -static phys_addr_t dw_pcie_ep_align_addr(struct pci_epc *epc, > - phys_addr_t pci_addr, size_t *pci_size, size_t *offset) > +static u64 dw_pcie_ep_align_addr(struct pci_epc *epc, u64 pci_addr, > + size_t *pci_size, size_t *offset) > { > struct dw_pcie_ep *ep = epc_get_drvdata(epc); > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > - phys_addr_t mask = pci->region_align - 1; > + u64 mask = pci->region_align - 1; > size_t ofst = pci_addr & mask; > > *pci_size = ALIGN(ofst + *pci_size, ep->page_size); > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index f4b8dc37e447..de8cc3658220 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -49,10 +49,10 @@ pci_epc_interface_string(enum pci_epc_interface_type type) > * @virt_addr: virtual address at which @pci_addr is mapped > */ > struct pci_epc_map { > - phys_addr_t pci_addr; > + u64 pci_addr; > size_t pci_size; > > - phys_addr_t map_pci_addr; > + u64 map_pci_addr; > size_t map_size; > > phys_addr_t phys_base; > @@ -93,8 +93,8 @@ struct pci_epc_ops { > struct pci_epf_bar *epf_bar); > void (*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > struct pci_epf_bar *epf_bar); > - phys_addr_t (*align_addr)(struct pci_epc *epc, phys_addr_t pci_addr, > - size_t *size, size_t *offset); > + u64 (*align_addr)(struct pci_epc *epc, u64 pci_addr, size_t *size, > + size_t *offset); > int (*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > phys_addr_t addr, u64 pci_addr, size_t size); > void (*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > -- > 2.47.0 >
On Tue, Oct 15, 2024 at 06:07:12PM +0900, Damien Le Moal wrote: > The PCI endpoint controller operation interface for the align_addr() > operation uses the phys_addr_t type for the PCI address argument and > return a value using this type. This is not ideal as PCI addresses are > bus addresses, not regular memory physical addresses. Replace the use of > phys_addr_t for this operation with the generic u64 type. To be > consistent with this change the Designware driver implementation of this > operation (function dw_pcie_ep_align_addr()) as well as the type of PCI > address fields of struct pci_epc_map are also changed. > > Fixes: e98c99e2ccad ("PCI: endpoint: Introduce pci_epc_mem_map()/unmap()") > Fixes: cb6b7158fdf5 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation") > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> I thought of applying it, but then decided to squash it with the offending patches. - Mani > --- > Changes from v1: > - Also updated the type of the pci_addr and map_pci_addr fields of > struct pci_epc_map. > > drivers/pci/controller/dwc/pcie-designware-ep.c | 6 +++--- > include/linux/pci-epc.h | 8 ++++---- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 0ada60487c25..df1460ed63d9 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -268,12 +268,12 @@ static int dw_pcie_find_index(struct dw_pcie_ep *ep, phys_addr_t addr, > return -EINVAL; > } > > -static phys_addr_t dw_pcie_ep_align_addr(struct pci_epc *epc, > - phys_addr_t pci_addr, size_t *pci_size, size_t *offset) > +static u64 dw_pcie_ep_align_addr(struct pci_epc *epc, u64 pci_addr, > + size_t *pci_size, size_t *offset) > { > struct dw_pcie_ep *ep = epc_get_drvdata(epc); > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > - phys_addr_t mask = pci->region_align - 1; > + u64 mask = pci->region_align - 1; > size_t ofst = pci_addr & mask; > > *pci_size = ALIGN(ofst + *pci_size, ep->page_size); > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index f4b8dc37e447..de8cc3658220 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -49,10 +49,10 @@ pci_epc_interface_string(enum pci_epc_interface_type type) > * @virt_addr: virtual address at which @pci_addr is mapped > */ > struct pci_epc_map { > - phys_addr_t pci_addr; > + u64 pci_addr; > size_t pci_size; > > - phys_addr_t map_pci_addr; > + u64 map_pci_addr; > size_t map_size; > > phys_addr_t phys_base; > @@ -93,8 +93,8 @@ struct pci_epc_ops { > struct pci_epf_bar *epf_bar); > void (*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > struct pci_epf_bar *epf_bar); > - phys_addr_t (*align_addr)(struct pci_epc *epc, phys_addr_t pci_addr, > - size_t *size, size_t *offset); > + u64 (*align_addr)(struct pci_epc *epc, u64 pci_addr, size_t *size, > + size_t *offset); > int (*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > phys_addr_t addr, u64 pci_addr, size_t size); > void (*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > -- > 2.47.0 >
On 10/17/24 01:59, Manivannan Sadhasivam wrote: > On Tue, Oct 15, 2024 at 06:07:12PM +0900, Damien Le Moal wrote: >> The PCI endpoint controller operation interface for the align_addr() >> operation uses the phys_addr_t type for the PCI address argument and >> return a value using this type. This is not ideal as PCI addresses are >> bus addresses, not regular memory physical addresses. Replace the use of >> phys_addr_t for this operation with the generic u64 type. To be >> consistent with this change the Designware driver implementation of this >> operation (function dw_pcie_ep_align_addr()) as well as the type of PCI >> address fields of struct pci_epc_map are also changed. >> >> Fixes: e98c99e2ccad ("PCI: endpoint: Introduce pci_epc_mem_map()/unmap()") >> Fixes: cb6b7158fdf5 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation") >> Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > > I thought of applying it, but then decided to squash it with the offending > patches. Fine with me. Thanks !
On 10/17/24 09:37, Damien Le Moal wrote: > On 10/17/24 01:59, Manivannan Sadhasivam wrote: >> On Tue, Oct 15, 2024 at 06:07:12PM +0900, Damien Le Moal wrote: >>> The PCI endpoint controller operation interface for the align_addr() >>> operation uses the phys_addr_t type for the PCI address argument and >>> return a value using this type. This is not ideal as PCI addresses are >>> bus addresses, not regular memory physical addresses. Replace the use of >>> phys_addr_t for this operation with the generic u64 type. To be >>> consistent with this change the Designware driver implementation of this >>> operation (function dw_pcie_ep_align_addr()) as well as the type of PCI >>> address fields of struct pci_epc_map are also changed. >>> >>> Fixes: e98c99e2ccad ("PCI: endpoint: Introduce pci_epc_mem_map()/unmap()") >>> Fixes: cb6b7158fdf5 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation") >>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org> >> >> I thought of applying it, but then decided to squash it with the offending >> patches. > > Fine with me. Thanks ! Just checked the pci/endpoint branch but I do not see this patch squashed there. Did you push the change ?
Hello, [...] > >> I thought of applying it, but then decided to squash it with the offending > >> patches. > > > > Fine with me. Thanks ! > > Just checked the pci/endpoint branch but I do not see this patch squashed there. If you so happen to check using the Git web interface kernel.org, then there is some sort of a caching going on that can return old results for a while before the cache expires. > Did you push the change ? Looks like changes are there. Krzysztof
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 0ada60487c25..df1460ed63d9 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -268,12 +268,12 @@ static int dw_pcie_find_index(struct dw_pcie_ep *ep, phys_addr_t addr, return -EINVAL; } -static phys_addr_t dw_pcie_ep_align_addr(struct pci_epc *epc, - phys_addr_t pci_addr, size_t *pci_size, size_t *offset) +static u64 dw_pcie_ep_align_addr(struct pci_epc *epc, u64 pci_addr, + size_t *pci_size, size_t *offset) { struct dw_pcie_ep *ep = epc_get_drvdata(epc); struct dw_pcie *pci = to_dw_pcie_from_ep(ep); - phys_addr_t mask = pci->region_align - 1; + u64 mask = pci->region_align - 1; size_t ofst = pci_addr & mask; *pci_size = ALIGN(ofst + *pci_size, ep->page_size); diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h index f4b8dc37e447..de8cc3658220 100644 --- a/include/linux/pci-epc.h +++ b/include/linux/pci-epc.h @@ -49,10 +49,10 @@ pci_epc_interface_string(enum pci_epc_interface_type type) * @virt_addr: virtual address at which @pci_addr is mapped */ struct pci_epc_map { - phys_addr_t pci_addr; + u64 pci_addr; size_t pci_size; - phys_addr_t map_pci_addr; + u64 map_pci_addr; size_t map_size; phys_addr_t phys_base; @@ -93,8 +93,8 @@ struct pci_epc_ops { struct pci_epf_bar *epf_bar); void (*clear_bar)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, struct pci_epf_bar *epf_bar); - phys_addr_t (*align_addr)(struct pci_epc *epc, phys_addr_t pci_addr, - size_t *size, size_t *offset); + u64 (*align_addr)(struct pci_epc *epc, u64 pci_addr, size_t *size, + size_t *offset); int (*map_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no, phys_addr_t addr, u64 pci_addr, size_t size); void (*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
The PCI endpoint controller operation interface for the align_addr() operation uses the phys_addr_t type for the PCI address argument and return a value using this type. This is not ideal as PCI addresses are bus addresses, not regular memory physical addresses. Replace the use of phys_addr_t for this operation with the generic u64 type. To be consistent with this change the Designware driver implementation of this operation (function dw_pcie_ep_align_addr()) as well as the type of PCI address fields of struct pci_epc_map are also changed. Fixes: e98c99e2ccad ("PCI: endpoint: Introduce pci_epc_mem_map()/unmap()") Fixes: cb6b7158fdf5 ("PCI: dwc: endpoint: Implement the pci_epc_ops::align_addr() operation") Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- Changes from v1: - Also updated the type of the pci_addr and map_pci_addr fields of struct pci_epc_map. drivers/pci/controller/dwc/pcie-designware-ep.c | 6 +++--- include/linux/pci-epc.h | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-)