| Message ID | 20240424-rockchip-pcie-ep-v1-v1-10-b1a02ddad650@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI: dw-rockchip: Add endpoint mode support | expand |
On Wed, Apr 24, 2024 at 05:16:28PM +0200, Niklas Cassel wrote: > Rockchip rk3588 requires 64k alignment. > While there is an existing device_id:vendor_id in the driver with 64k > alignment, that device_id:vendor_id is am654, which uses BAR2 instead of > BAR0 as the test_reg_bar, and also has special is_am654_pci_dev() checks > in the driver to disallow BAR0. In order to allow testing all BARs, add a > new rk3588 entry in the driver. > > We intentionally do not add the vendor id to pci_ids.h, since the policy > for that file is that the vendor id has to be used by multiple drivers. > > Hopefully, this new entry will be short-lived, as there is a series on the > mailing list which intends to move the address alignment restrictions from > this driver to the endpoint side. > > Add a new entry for rk3588 in order to allow us to test all BARs. > > Signed-off-by: Niklas Cassel <cassel@kernel.org> > --- > drivers/misc/pci_endpoint_test.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > index c38a6083f0a7..a7f593b4e3b3 100644 > --- a/drivers/misc/pci_endpoint_test.c > +++ b/drivers/misc/pci_endpoint_test.c > @@ -84,6 +84,9 @@ > #define PCI_DEVICE_ID_RENESAS_R8A774E1 0x0025 > #define PCI_DEVICE_ID_RENESAS_R8A779F0 0x0031 > > +#define PCI_VENDOR_ID_ROCKCHIP 0x1d87 > +#define PCI_DEVICE_ID_ROCKCHIP_RK3588 0x3588 > + Did you make sure 0x3588 will not used by other production with vendor id 0x1d87? Frank > static DEFINE_IDA(pci_endpoint_test_ida); > > #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \ > @@ -980,6 +983,11 @@ static const struct pci_endpoint_test_data j721e_data = { > .irq_type = IRQ_TYPE_MSI, > }; > > +static const struct pci_endpoint_test_data rk3588_data = { > + .alignment = SZ_64K, > + .irq_type = IRQ_TYPE_MSI, > +}; > + > static const struct pci_device_id pci_endpoint_test_tbl[] = { > { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_DRA74x), > .driver_data = (kernel_ulong_t)&default_data, > @@ -1017,6 +1025,9 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = { > { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_J721S2), > .driver_data = (kernel_ulong_t)&j721e_data, > }, > + { PCI_DEVICE(PCI_VENDOR_ID_ROCKCHIP, PCI_DEVICE_ID_ROCKCHIP_RK3588), > + .driver_data = (kernel_ulong_t)&rk3588_data, > + }, > { } > }; > MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl); > > -- > 2.44.0 >
On Thu, Apr 25, 2024 at 03:16:53PM -0400, Frank Li wrote: > On Wed, Apr 24, 2024 at 05:16:28PM +0200, Niklas Cassel wrote: > > Rockchip rk3588 requires 64k alignment. > > While there is an existing device_id:vendor_id in the driver with 64k > > alignment, that device_id:vendor_id is am654, which uses BAR2 instead of > > BAR0 as the test_reg_bar, and also has special is_am654_pci_dev() checks > > in the driver to disallow BAR0. In order to allow testing all BARs, add a > > new rk3588 entry in the driver. > > > > We intentionally do not add the vendor id to pci_ids.h, since the policy > > for that file is that the vendor id has to be used by multiple drivers. > > > > Hopefully, this new entry will be short-lived, as there is a series on the > > mailing list which intends to move the address alignment restrictions from > > this driver to the endpoint side. > > > > Add a new entry for rk3588 in order to allow us to test all BARs. > > > > Signed-off-by: Niklas Cassel <cassel@kernel.org> > > --- > > drivers/misc/pci_endpoint_test.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > > index c38a6083f0a7..a7f593b4e3b3 100644 > > --- a/drivers/misc/pci_endpoint_test.c > > +++ b/drivers/misc/pci_endpoint_test.c > > @@ -84,6 +84,9 @@ > > #define PCI_DEVICE_ID_RENESAS_R8A774E1 0x0025 > > #define PCI_DEVICE_ID_RENESAS_R8A779F0 0x0031 > > > > +#define PCI_VENDOR_ID_ROCKCHIP 0x1d87 > > +#define PCI_DEVICE_ID_ROCKCHIP_RK3588 0x3588 > > + > > Did you make sure 0x3588 will not used by other production with vendor id > 0x1d87? Hello Frank, I do not fully understand your question. Vendor ID 0x1d87 is rockchip: https://admin.pci-ids.ucw.cz/read/PC/1d87 https://admin.pci-ids.ucw.cz/read/PC/1d87/3588 is RK3588. This is the PCI device ID and the vendor ID that the device will have on reset. So since rockchip has put these values as the reset values, and they appear in the PCI IDE repository, I assume that they should be fine to use. I could not find any other Linux device driver using this PCI device and vendor ID, if that was what you meant. Kind regards, Niklas
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c index c38a6083f0a7..a7f593b4e3b3 100644 --- a/drivers/misc/pci_endpoint_test.c +++ b/drivers/misc/pci_endpoint_test.c @@ -84,6 +84,9 @@ #define PCI_DEVICE_ID_RENESAS_R8A774E1 0x0025 #define PCI_DEVICE_ID_RENESAS_R8A779F0 0x0031 +#define PCI_VENDOR_ID_ROCKCHIP 0x1d87 +#define PCI_DEVICE_ID_ROCKCHIP_RK3588 0x3588 + static DEFINE_IDA(pci_endpoint_test_ida); #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \ @@ -980,6 +983,11 @@ static const struct pci_endpoint_test_data j721e_data = { .irq_type = IRQ_TYPE_MSI, }; +static const struct pci_endpoint_test_data rk3588_data = { + .alignment = SZ_64K, + .irq_type = IRQ_TYPE_MSI, +}; + static const struct pci_device_id pci_endpoint_test_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_DRA74x), .driver_data = (kernel_ulong_t)&default_data, @@ -1017,6 +1025,9 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_J721S2), .driver_data = (kernel_ulong_t)&j721e_data, }, + { PCI_DEVICE(PCI_VENDOR_ID_ROCKCHIP, PCI_DEVICE_ID_ROCKCHIP_RK3588), + .driver_data = (kernel_ulong_t)&rk3588_data, + }, { } }; MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl);
Rockchip rk3588 requires 64k alignment. While there is an existing device_id:vendor_id in the driver with 64k alignment, that device_id:vendor_id is am654, which uses BAR2 instead of BAR0 as the test_reg_bar, and also has special is_am654_pci_dev() checks in the driver to disallow BAR0. In order to allow testing all BARs, add a new rk3588 entry in the driver. We intentionally do not add the vendor id to pci_ids.h, since the policy for that file is that the vendor id has to be used by multiple drivers. Hopefully, this new entry will be short-lived, as there is a series on the mailing list which intends to move the address alignment restrictions from this driver to the endpoint side. Add a new entry for rk3588 in order to allow us to test all BARs. Signed-off-by: Niklas Cassel <cassel@kernel.org> --- drivers/misc/pci_endpoint_test.c | 11 +++++++++++ 1 file changed, 11 insertions(+)