| Message ID | 20231227182727.1747435-5-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: imx6: Clean up and add imx95 pci support | expand |
On Wed, Dec 27, 2023 at 01:27:15PM -0500, Frank Li wrote: > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > proptery for iMX8MQ to indicate pci controller index. > property > This adjustment paves the way for eliminating the hardcoded check on the > base register for acquiring the controller_id. > > ... > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > imx6_pcie->controller_id = 1; > ... > > The controller_id is crucial and utilized for certain register bit > positions. It must align precisely with the controller index in the SoC. > An auto-incremented ID don't fit this case. The DTS or fuse configurations > may deactivate specific PCI controllers. > You cannot change the binding for the sake of driver. But you can make this change in other way. See below... > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > Notes: > Change from v5 to v6 > - rework commit message to explain why need required and why auto increase > id not work > > Change from v4 to v5 > - new patch at v5 > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > index d91b639ae7ae7..8f39b4e6e8491 100644 > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > @@ -265,6 +265,17 @@ allOf: > - const: apps > - const: turnoff > > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx8mq-pcie > + - fsl,imx8mq-pcie-ep "linux,pci-domain" is a generic property. So you cannot make it required only for certain SoCs. But you can make it so for all SoCs. This way, the drivers can also rely on it. Now, you should get rid of the commit message about driver internals: > This adjustment paves the way for eliminating the hardcoded check on the > base register for acquiring the controller_id. > > ... > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > imx6_pcie->controller_id = 1; > ... > > The controller_id is crucial and utilized for certain register bit > positions. It must align precisely with the controller index in the SoC. > An auto-incremented ID don't fit this case. The DTS or fuse configurations > may deactivate specific PCI controllers. > - Mani > + then: > + required: > + - linux,pci-domain > + > additionalProperties: true > > ... > -- > 2.34.1 >
On Sun, Jan 07, 2024 at 08:45:06AM +0530, Manivannan Sadhasivam wrote: > On Wed, Dec 27, 2023 at 01:27:15PM -0500, Frank Li wrote: > > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > > proptery for iMX8MQ to indicate pci controller index. > > > > property > > > This adjustment paves the way for eliminating the hardcoded check on the > > base register for acquiring the controller_id. > > > > ... > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > imx6_pcie->controller_id = 1; > > ... > > > > The controller_id is crucial and utilized for certain register bit > > positions. It must align precisely with the controller index in the SoC. > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > may deactivate specific PCI controllers. > > > > You cannot change the binding for the sake of driver. But you can make this > change in other way. See below... > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > > > Notes: > > Change from v5 to v6 > > - rework commit message to explain why need required and why auto increase > > id not work > > > > Change from v4 to v5 > > - new patch at v5 > > > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > index d91b639ae7ae7..8f39b4e6e8491 100644 > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > @@ -265,6 +265,17 @@ allOf: > > - const: apps > > - const: turnoff > > > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8mq-pcie > > + - fsl,imx8mq-pcie-ep > > "linux,pci-domain" is a generic property. So you cannot make it required only > for certain SoCs. Sorry, why not? there are many generic property. > But you can make it so for all SoCs. This way, the drivers > can also rely on it. > > Now, you should get rid of the commit message about driver internals: Not all dts already added "linux,pci-domain" yet. If required for all SOCs, it will cause dtb check warnings. Frank > > > This adjustment paves the way for eliminating the hardcoded check on the > > base register for acquiring the controller_id. > > > > ... > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > imx6_pcie->controller_id = 1; > > ... > > > > The controller_id is crucial and utilized for certain register bit > > positions. It must align precisely with the controller index in the SoC. > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > may deactivate specific PCI controllers. > > > > - Mani > > > + then: > > + required: > > + - linux,pci-domain > > + > > additionalProperties: true > > > > ... > > -- > > 2.34.1 > > > > -- > மணிவண்ணன் சதாசிவம்
On Sat, Jan 06, 2024 at 11:47:36PM -0500, Frank Li wrote: > On Sun, Jan 07, 2024 at 08:45:06AM +0530, Manivannan Sadhasivam wrote: > > On Wed, Dec 27, 2023 at 01:27:15PM -0500, Frank Li wrote: > > > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > > > proptery for iMX8MQ to indicate pci controller index. > > > > > > > property > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > base register for acquiring the controller_id. > > > > > > ... > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > imx6_pcie->controller_id = 1; > > > ... > > > > > > The controller_id is crucial and utilized for certain register bit > > > positions. It must align precisely with the controller index in the SoC. > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > may deactivate specific PCI controllers. > > > > > > > You cannot change the binding for the sake of driver. But you can make this > > change in other way. See below... > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > > > > Notes: > > > Change from v5 to v6 > > > - rework commit message to explain why need required and why auto increase > > > id not work > > > > > > Change from v4 to v5 > > > - new patch at v5 > > > > > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > index d91b639ae7ae7..8f39b4e6e8491 100644 > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > @@ -265,6 +265,17 @@ allOf: > > > - const: apps > > > - const: turnoff > > > > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + enum: > > > + - fsl,imx8mq-pcie > > > + - fsl,imx8mq-pcie-ep > > > > "linux,pci-domain" is a generic property. So you cannot make it required only > > for certain SoCs. > > Sorry, why not? there are many generic property. > It doesn't make sense to make it required only for specific SoCs since it is not specific to any SoC. You can make it required for all. > > But you can make it so for all SoCs. This way, the drivers > > can also rely on it. > > > > Now, you should get rid of the commit message about driver internals: > > Not all dts already added "linux,pci-domain" yet. If required for all SOCs, > it will cause dtb check warnings. > You can safely add this property to all DTS. Nothing will break. - Mani > Frank > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > base register for acquiring the controller_id. > > > > > > ... > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > imx6_pcie->controller_id = 1; > > > ... > > > > > > The controller_id is crucial and utilized for certain register bit > > > positions. It must align precisely with the controller index in the SoC. > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > may deactivate specific PCI controllers. > > > > > > > - Mani > > > > > + then: > > > + required: > > > + - linux,pci-domain > > > + > > > additionalProperties: true > > > > > > ... > > > -- > > > 2.34.1 > > > > > > > -- > > மணிவண்ணன் சதாசிவம்
On Sun, Jan 07, 2024 at 10:49:17AM +0530, Manivannan Sadhasivam wrote: > On Sat, Jan 06, 2024 at 11:47:36PM -0500, Frank Li wrote: > > On Sun, Jan 07, 2024 at 08:45:06AM +0530, Manivannan Sadhasivam wrote: > > > On Wed, Dec 27, 2023 at 01:27:15PM -0500, Frank Li wrote: > > > > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > > > > proptery for iMX8MQ to indicate pci controller index. > > > > > > > > > > property > > > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > > base register for acquiring the controller_id. > > > > > > > > ... > > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > > imx6_pcie->controller_id = 1; > > > > ... > > > > > > > > The controller_id is crucial and utilized for certain register bit > > > > positions. It must align precisely with the controller index in the SoC. > > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > > may deactivate specific PCI controllers. > > > > > > > > > > You cannot change the binding for the sake of driver. But you can make this > > > change in other way. See below... > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > --- > > > > > > > > Notes: > > > > Change from v5 to v6 > > > > - rework commit message to explain why need required and why auto increase > > > > id not work > > > > > > > > Change from v4 to v5 > > > > - new patch at v5 > > > > > > > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > > > > 1 file changed, 11 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > index d91b639ae7ae7..8f39b4e6e8491 100644 > > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > @@ -265,6 +265,17 @@ allOf: > > > > - const: apps > > > > - const: turnoff > > > > > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > + - fsl,imx8mq-pcie > > > > + - fsl,imx8mq-pcie-ep > > > > > > "linux,pci-domain" is a generic property. So you cannot make it required only > > > for certain SoCs. > > > > Sorry, why not? there are many generic property. > > > > It doesn't make sense to make it required only for specific SoCs since it is not > specific to any SoC. You can make it required for all. More than 2 controller need require "linux,pci-domain". > > > > But you can make it so for all SoCs. This way, the drivers > > > can also rely on it. > > > > > > Now, you should get rid of the commit message about driver internals: > > > > Not all dts already added "linux,pci-domain" yet. If required for all SOCs, > > it will cause dtb check warnings. > > > > You can safely add this property to all DTS. Nothing will break. Yes, but it will be off topic of this patch serial. I can submit new patches for this later. After all dts changed, then I remove this conditional check. This patch serial is already quite big, (17 patches). And I don't want to involve new DTB check warning. Frank > > - Mani > > > Frank > > > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > > base register for acquiring the controller_id. > > > > > > > > ... > > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > > imx6_pcie->controller_id = 1; > > > > ... > > > > > > > > The controller_id is crucial and utilized for certain register bit > > > > positions. It must align precisely with the controller index in the SoC. > > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > > may deactivate specific PCI controllers. > > > > > > > > > > - Mani > > > > > > > + then: > > > > + required: > > > > + - linux,pci-domain > > > > + > > > > additionalProperties: true > > > > > > > > ... > > > > -- > > > > 2.34.1 > > > > > > > > > > -- > > > மணிவண்ணன் சதாசிவம் > > -- > மணிவண்ணன் சதாசிவம்
On Sun, Jan 07, 2024 at 12:38:10AM -0500, Frank Li wrote: > On Sun, Jan 07, 2024 at 10:49:17AM +0530, Manivannan Sadhasivam wrote: > > On Sat, Jan 06, 2024 at 11:47:36PM -0500, Frank Li wrote: > > > On Sun, Jan 07, 2024 at 08:45:06AM +0530, Manivannan Sadhasivam wrote: > > > > On Wed, Dec 27, 2023 at 01:27:15PM -0500, Frank Li wrote: > > > > > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > > > > > proptery for iMX8MQ to indicate pci controller index. > > > > > > > > > > > > > property > > > > > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > > > base register for acquiring the controller_id. > > > > > > > > > > ... > > > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > > > imx6_pcie->controller_id = 1; > > > > > ... > > > > > > > > > > The controller_id is crucial and utilized for certain register bit > > > > > positions. It must align precisely with the controller index in the SoC. > > > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > > > may deactivate specific PCI controllers. > > > > > > > > > > > > > You cannot change the binding for the sake of driver. But you can make this > > > > change in other way. See below... > > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > --- > > > > > > > > > > Notes: > > > > > Change from v5 to v6 > > > > > - rework commit message to explain why need required and why auto increase > > > > > id not work > > > > > > > > > > Change from v4 to v5 > > > > > - new patch at v5 > > > > > > > > > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > > > > > 1 file changed, 11 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > > index d91b639ae7ae7..8f39b4e6e8491 100644 > > > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > > @@ -265,6 +265,17 @@ allOf: > > > > > - const: apps > > > > > - const: turnoff > > > > > > > > > > + - if: > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - fsl,imx8mq-pcie > > > > > + - fsl,imx8mq-pcie-ep > > > > > > > > "linux,pci-domain" is a generic property. So you cannot make it required only > > > > for certain SoCs. > > > > > > Sorry, why not? there are many generic property. > > > > > > > It doesn't make sense to make it required only for specific SoCs since it is not > > specific to any SoC. You can make it required for all. > > More than 2 controller need require "linux,pci-domain". > But this property is applicable to single controller also. > > > > > > But you can make it so for all SoCs. This way, the drivers > > > > can also rely on it. > > > > > > > > Now, you should get rid of the commit message about driver internals: > > > > > > Not all dts already added "linux,pci-domain" yet. If required for all SOCs, > > > it will cause dtb check warnings. > > > > > > > You can safely add this property to all DTS. Nothing will break. > > Yes, but it will be off topic of this patch serial. > > I can submit new patches for this later. After all dts changed, then I > remove this conditional check. > > This patch serial is already quite big, (17 patches). > > And I don't want to involve new DTB check warning. > Okay. But please follow up on this once this series gets merged. - Mani > Frank > > > > > - Mani > > > > > Frank > > > > > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > > > base register for acquiring the controller_id. > > > > > > > > > > ... > > > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > > > imx6_pcie->controller_id = 1; > > > > > ... > > > > > > > > > > The controller_id is crucial and utilized for certain register bit > > > > > positions. It must align precisely with the controller index in the SoC. > > > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > > > may deactivate specific PCI controllers. > > > > > > > > > > > > > - Mani > > > > > > > > > + then: > > > > > + required: > > > > > + - linux,pci-domain > > > > > + > > > > > additionalProperties: true > > > > > > > > > > ... > > > > > -- > > > > > 2.34.1 > > > > > > > > > > > > > -- > > > > மணிவண்ணன் சதாசிவம் > > > > -- > > மணிவண்ணன் சதாசிவம்
On Sun, Jan 07, 2024 at 11:59:11AM +0530, Manivannan Sadhasivam wrote: > On Sun, Jan 07, 2024 at 12:38:10AM -0500, Frank Li wrote: > > On Sun, Jan 07, 2024 at 10:49:17AM +0530, Manivannan Sadhasivam wrote: > > > On Sat, Jan 06, 2024 at 11:47:36PM -0500, Frank Li wrote: > > > > On Sun, Jan 07, 2024 at 08:45:06AM +0530, Manivannan Sadhasivam wrote: > > > > > On Wed, Dec 27, 2023 at 01:27:15PM -0500, Frank Li wrote: > > > > > > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > > > > > > proptery for iMX8MQ to indicate pci controller index. > > > > > > > > > > > > > > > > property > > > > > > > > > > > This adjustment paves the way for eliminating the hardcoded check on the > > > > > > base register for acquiring the controller_id. > > > > > > > > > > > > ... > > > > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > > > > > > imx6_pcie->controller_id = 1; > > > > > > ... > > > > > > > > > > > > The controller_id is crucial and utilized for certain register bit > > > > > > positions. It must align precisely with the controller index in the SoC. > > > > > > An auto-incremented ID don't fit this case. The DTS or fuse configurations > > > > > > may deactivate specific PCI controllers. > > > > > > > > > > > > > > > > You cannot change the binding for the sake of driver. But you can make this > > > > > change in other way. See below... > > > > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > > --- > > > > > > > > > > > > Notes: > > > > > > Change from v5 to v6 > > > > > > - rework commit message to explain why need required and why auto increase > > > > > > id not work > > > > > > > > > > > > Change from v4 to v5 > > > > > > - new patch at v5 > > > > > > > > > > > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > > > > > > 1 file changed, 11 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > > > index d91b639ae7ae7..8f39b4e6e8491 100644 > > > > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml > > > > > > @@ -265,6 +265,17 @@ allOf: > > > > > > - const: apps > > > > > > - const: turnoff > > > > > > > > > > > > + - if: > > > > > > + properties: > > > > > > + compatible: > > > > > > + contains: > > > > > > + enum: > > > > > > + - fsl,imx8mq-pcie > > > > > > + - fsl,imx8mq-pcie-ep > > > > > > > > > > "linux,pci-domain" is a generic property. So you cannot make it required only > > > > > for certain SoCs. > > > > > > > > Sorry, why not? there are many generic property. > > > > > > > > > > It doesn't make sense to make it required only for specific SoCs since it is not > > > specific to any SoC. You can make it required for all. > > > > More than 2 controller need require "linux,pci-domain". > > > > But this property is applicable to single controller also. Not really. I don't understand the issue. Some SoCs have a dependency on the numbering and need the property. Others don't. They just want (but don't need) consistent numbering. Rob
On Wed, 27 Dec 2023 13:27:15 -0500, Frank Li wrote: > iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required > proptery for iMX8MQ to indicate pci controller index. > > This adjustment paves the way for eliminating the hardcoded check on the > base register for acquiring the controller_id. > > ... > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) > imx6_pcie->controller_id = 1; > ... > > The controller_id is crucial and utilized for certain register bit > positions. It must align precisely with the controller index in the SoC. > An auto-incremented ID don't fit this case. The DTS or fuse configurations > may deactivate specific PCI controllers. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > Notes: > Change from v5 to v6 > - rework commit message to explain why need required and why auto increase > id not work > > Change from v4 to v5 > - new patch at v5 > > .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml index d91b639ae7ae7..8f39b4e6e8491 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml @@ -265,6 +265,17 @@ allOf: - const: apps - const: turnoff + - if: + properties: + compatible: + contains: + enum: + - fsl,imx8mq-pcie + - fsl,imx8mq-pcie-ep + then: + required: + - linux,pci-domain + additionalProperties: true ...
iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required proptery for iMX8MQ to indicate pci controller index. This adjustment paves the way for eliminating the hardcoded check on the base register for acquiring the controller_id. ... if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) imx6_pcie->controller_id = 1; ... The controller_id is crucial and utilized for certain register bit positions. It must align precisely with the controller index in the SoC. An auto-incremented ID don't fit this case. The DTS or fuse configurations may deactivate specific PCI controllers. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Notes: Change from v5 to v6 - rework commit message to explain why need required and why auto increase id not work Change from v4 to v5 - new patch at v5 .../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+)