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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d7b26561b60f8aee2674dfebc45856d7fba305eb.camel@mediatek.com>
Date: Wed, 8 Jan 2025 07:30:24 +0000
From: Jianjun Wang (王建军) <Jianjun.Wang@...iatek.com>
To: "krzk@...nel.org" <krzk@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "manivannan.sadhasivam@...aro.org"
	<manivannan.sadhasivam@...aro.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "robh@...nel.org" <robh@...nel.org>, "kw@...ux.com"
	<kw@...ux.com>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, Xavier Chang (張獻文)
	<Xavier.Chang@...iatek.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	Ryder Lee <Ryder.Lee@...iatek.com>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH 1/5] dt-bindings: PCI: mediatek-gen3: Add MT8196 support

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
>