Re: [PATCH v7 04/16] dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

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
> 

-- 
மணிவண்ணன் சதாசிவம்