Re: [PATCH v2 3/4] dt-bindings: PCI: Add StarFive JH7110 PCIe controller

[Conor Dooley]

On Mon, Aug 07, 2023 at 02:54:50PM +0800, Minda Chen wrote:
> On 2023/8/4 15:10, Conor Dooley wrote:
> > On Thu, Jul 27, 2023 at 06:39:48PM +0800, Minda Chen wrote:
> >> +  starfive,stg-syscon:
> >> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> >> +    items:
> >> +      - items:
> >> +          - description: phandle to System Register Controller stg_syscon node.
> >> +          - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> >> +          - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> >> +          - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> >> +          - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe.
> >> +    description:
> >> +      The phandle to System Register Controller syscon node and the offset
> >> +      of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset
> >> +      for PCIe.
> > 
> > These property names tie them closely with naming on the jh7110, but
> > there's little value in specifying all of these offsets when you have
> > one implementation where they are all fixed.
> Yes, the offset value is tied to SoC. 
> > Do you know what the jh81xx stuff is going to do yet w.r.t. PCI and if
> > so, how could you reuse this property?
> I do not participate in jh8100. But I heard sys-syscon is exist in 81xx.
> But I think stg-syscon and sys-syscon  can be move to a common dt-binding doc.
> Bot 71x0 and 81x0 driver can use this. 
> > Particularly, saying "register 0" seems unlikely to transfer well
> > between SoCs.
> > I'd be inclined to drop the offsets entirely & rely on match data to
> > provide them if needed in the future.
> That's ok. The dts can change to starfive,stg-syscon = <&stg_syscon>;
> I will try to move the offset to driver match data.

To be clear, you don't need match data now, only when the jh8100 stuff
shows up. Until then the values can be hard coded in the driver as there
is only one device it works for.

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature