Re: [PATCH 1/5] dt-bindings: PCI: mediatek-gen3: Add MT8196 support

["Jianjun Wang (王建军)" <Jianjun.Wang@mediatek.com>]

On Wed, 2025-01-08 at 08:16 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 07/01/2025 09:43, Jianjun Wang (王建军) wrote:
> > On Mon, 2025-01-06 at 13:27 +0100, Krzysztof Kozlowski wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On 06/01/2025 10:26, Jianjun Wang (王建军) wrote:
> > > > On Fri, 2025-01-03 at 10:10 +0100, Krzysztof Kozlowski wrote:
> > > > > External email : Please do not click links or open
> > > > > attachments
> > > > > until
> > > > > you have verified the sender or the content.
> > > > > 
> > > > > 
> > > > > On Fri, Jan 03, 2025 at 02:00:11PM +0800, Jianjun Wang wrote:
> > > > > > +        clock-names:
> > > > > > +          items:
> > > > > > +            - const: pl_250m
> > > > > > +            - const: tl_26m
> > > > > > +            - const: peri_26m
> > > > > > +            - const: peri_mem
> > > > > > +            - const: ahb_apb
> > > > > > +            - const: low_power
> > > > > > +
> > > > > > +        resets:
> > > > > > +          minItems: 1
> > > > > > +          maxItems: 2
> > > > > > +
> > > > > > +        reset-names:
> > > > > > +          minItems: 1
> > > > > > +          maxItems: 2
> > > > > 
> > > > > Why resets are flexible?
> > > > 
> > > > There are two resets, one for MAC and another for PHY, some
> > > > platforms
> > > > may only use one of them.
> > > 
> > > Even more questions. What does it mean use? Is it there or is it
> > > not?
> > 
> > It will be used by calling the reset controller's APIs in the PCIe
> > controller driver. Ideally, it should be de-asserted before PCIe
> > initialization and should be asserted if PCIe powers down or the
> > driver
> > is removed.
> 
> So it is there? Then drop minItems.
> 
> > 
> > > Platform like SoC? But this is one specific SoC, it cannot be
> > > used on
> > > different SoC.
> > 
> > Yes, it should be SoC, each SoC have its own resets, and the number
> > of
> > resets for each SoC is defined by the hardware design, most SoCs
> > should
> > have one reset for MAC and one reset for PHY.
> 
> You respond with some obvious things, so this review won't work.
> Properties are supposed to be constrained. Your arguments that
> something
> else has something else, do not apply. It does not matter what
> something
> else has.
> 
> > 
> > > 
> > > > 
> > > > Would you prefer to set the number of resets to a fixed value
> > > > for
> > > > specific platforms?
> > > 
> > > Everything should be constrained to match hardware.
> > 
> > For MT8196, there are 2 resets. Should I use a fixed item in this
> > case?
> 
> Yes. I asked why this soc has this flexible and you speak about some
> other socs.

Got it, sorry for the noise, will fix it in the next version.

Thanks.

> 
> 
> 
> Best regards,
> Krzysztof
>