Received-SPF: pass (google.com: domain of linux-kernel+bounces-44822-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 30 Jan 2024 10:20:30 -0500
From: Frank Li <Frank.li@nxp.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Conor Dooley <conor@kernel.org>, thinh.nguyen@synopsys.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, balbi@kernel.org, devicetree@vger.kernel.org,
	gregkh@linuxfoundation.org, imx@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	mark.rutland@arm.com, mathias.nyman@intel.com, pku.leo@gmail.com,
	sergei.shtylyov@cogentembedded.com
Subject: Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info
Message-ID: <ZbkTvu0Q85zyieXr@lizhi-Precision-Tower-5810>
References: <20240123-anew-lilly-0d645bdbfb30@spud>
 <Za//LX9U6QG5A5NW@lizhi-Precision-Tower-5810>
 <20240123-nanometer-atlantic-6465b270043a@spud>
 <ZbAR/NQvjUnf2At+@lizhi-Precision-Tower-5810>
 <46781012-2678-4f6c-9aee-b020cabcbb28@linaro.org>
 <ZbA8ea9Ex+hMdDDZ@lizhi-Precision-Tower-5810>
 <ZbfB/KT+fzO/F2e5@lizhi-Precision-Tower-5810>
 <20240129-encode-catchable-f5712d561a47@spud>
 <ZbfjZoHiH7BsKyzl@lizhi-Precision-Tower-5810>
 <f3811c1f-eff2-4c7b-8cea-6d3115525235@linaro.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f3811c1f-eff2-4c7b-8cea-6d3115525235@linaro.org>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?fWeL7MID7lqmFTEcuSydK7clmyymbSiMAILX3UFEhSQ+rgQ67RZEXCnVFwlj?=
 =?us-ascii?Q?xzCBBUoiNDaFDSWu9DFCEl5kQWxSAUefnqduD/Xif3FI6dCYY5QhdQAzJheJ?=
 =?us-ascii?Q?ITbWVYautJ1w1t1eyIGcnuXFpVtbJEYHi5yfHb1zUxUQTAjeUYKN/f5/n92w?=
 =?us-ascii?Q?TY3iZXPumZAzDDsZu02ntY/1HoBPxF33Aml2MMiCnWWnrzlHMlGQxiiSMAKT?=
 =?us-ascii?Q?Z99DT23WKJaUJi+gTmSDOCgo8a7LAvn5JOvvEe6MgWQLPtcx7+Ez5b87QzSG?=
 =?us-ascii?Q?yDoIpgGQnH9Qw8ThubdnbGypeF9h1Iw9wjYMgZj5C9KlqKQE2czaK5plWmid?=
 =?us-ascii?Q?Pb2/qPOEucGaF3QXhWfEhY0wtpFcOZ+eGmu4JqV5jt+I6rz5cddz5tjJwDhj?=
 =?us-ascii?Q?tCJwaebY8OdPpEM2czcLxzXW/9I3toGOr53NfiE5NKJvb0jxYa34c5n0TAwC?=
 =?us-ascii?Q?FUmzWGANjqizK0n6fcRyAU1UzWiGckT8mGZ7RPp8TTpio/R3FZADwH5hJ8BQ?=
 =?us-ascii?Q?mFgn/8KkEV2GV5qMfhwf4mAChbl4EduNPOd2U8akrHGjdRrYzAWblSpqtRwh?=
 =?us-ascii?Q?//X4D125vMlAgz/J1Il+Kq5DssvaAwbHwxw3aSj9zvCm1SlE3PWt8eyVUFD+?=
 =?us-ascii?Q?trziJ98AgWHHYTf+YwaDcfivjE/Ge0cHn7QFHQOm/w1O8uFVol7iSbupDKoz?=
 =?us-ascii?Q?LzUaJ64KdDHsomytm804P9Yy7uRdhMIA5y6Ju4O9Q+p3n+ivl1N16cBAuo0n?=
 =?us-ascii?Q?OufnxqvzsLAVYNkN0cmVtpibAiC6eMCufHi5RHg+Cqa2oFqVdFMyLb7RxJ5y?=
 =?us-ascii?Q?WE5D1zZccK/1pdIbdIq7Ul2gAdWUpseveN148ugaNOhgrRW4LocS5fBOMlhp?=
 =?us-ascii?Q?e/YJ0vwxCc7W0fEcCfvZiNvbk8t/q10Tn2r2+U0rAg+HoGclsy1Bb0gIiJgP?=
 =?us-ascii?Q?gVEuifxH/cZHXEccNKY7qZGeYfTSNyg2Cj4DwfIwcFuAzn5L7wyQeVLS+D8Z?=
 =?us-ascii?Q?60vFpSntcFiMCzjw/GmhEACYf8baLiTRfrGIc7xDdTOt6pbNX3ZV8UdmD1zp?=
 =?us-ascii?Q?rKcMKIpzM90sarZ287wFTJVDdIzjIe8qx/WET7dxO5nYenIIbb8YvvDYoUjk?=
 =?us-ascii?Q?KEdKp5oBH2IFpAq2m03kpr4xli2EdZw/trg3AgbyQed9RlLwxp5ADQGy5SwG?=
 =?us-ascii?Q?64EBfYMtzGIYWTPGbu9Qg5e/yeMoCaL70CQTb6juimalVBNd5OvZVvhzq9Ju?=
 =?us-ascii?Q?LJFpY6WrTj/zbsp2dr1uNm6WueL7fbrEewOWXs3ZtOvifSHdN912At0tA+mw?=
 =?us-ascii?Q?jodbTNRfSWCYZm/Svxfd6Or5f3Pnbjig2351GNpNprZMGO/hpgs+9Ie4ErEF?=
 =?us-ascii?Q?mSRxaQirnPKeAf/5oDTvEutUPkUwdqv3y0yf3+VqgAejVGtCDmZzgC4+ruJG?=
 =?us-ascii?Q?eFcCTuAIudCM14WSgtmR30PVZ+2uED8RP8lHFPR4nLgrvxzsWMM1ZJ/7/mqq?=
 =?us-ascii?Q?U+t++dkX6aNVwJu7mnUds9ghibothu2TcKHFmMWCi04D8xchfgNYD+XmtJOt?=
 =?us-ascii?Q?H/dZikZU9BaLJgr81OmqHqh+XHaLJraU423jtH6P?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 10611e75-de93-4bd4-624f-08dc21a70423
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 15:20:39.3871
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Zs+zM09xCfOAfm1A6Fq116/cWpXBTYVN/uTR6+AtcgbNguIQ5LHRX6FXE0VagyE98kFRWgDsNShy5TDv4fwBWw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB9929

On Tue, Jan 30, 2024 at 08:40:29AM +0100, Krzysztof Kozlowski wrote:
> On 29/01/2024 18:41, Frank Li wrote:
> > On Mon, Jan 29, 2024 at 04:49:21PM +0000, Conor Dooley wrote:
> >> On Mon, Jan 29, 2024 at 10:19:24AM -0500, Frank Li wrote:
> >>> On Tue, Jan 23, 2024 at 05:23:53PM -0500, Frank Li wrote:
> >>>> On Tue, Jan 23, 2024 at 10:46:39PM +0100, Krzysztof Kozlowski wrote:
> >>>>> On 23/01/2024 20:22, Frank Li wrote:
> >>>>>> On Tue, Jan 23, 2024 at 06:42:27PM +0000, Conor Dooley wrote:
> >>>>>>> On Tue, Jan 23, 2024 at 01:02:21PM -0500, Frank Li wrote:
> >>>>>>>> On Tue, Jan 23, 2024 at 05:51:48PM +0000, Conor Dooley wrote:
> >>>>>>>>> On Tue, Jan 23, 2024 at 12:49:27PM -0500, Frank Li wrote:
> >>>>>>>>>> On Tue, Jan 23, 2024 at 05:27:13PM +0000, Conor Dooley wrote:
> >>>>>>>>>>> On Tue, Jan 23, 2024 at 12:02:05PM -0500, Frank Li wrote:
> >>>>>>>>>>>> Add device tree binding allow platform overwrite default value of *REQIN in
> >>>>>>>>>>>> GSBUSCFG0.
> >>>>>>>>>>>
> >>>>>>>>>>> Why might a platform actually want to do this? Why does this need to be
> >>>>>>>>>>> set at the board level and being aware of which SoC is in use is not
> >>>>>>>>>>> sufficient for the driver to set the correct values?
> >>>>>>>>>>
> >>>>>>>>>> In snps,dwc3.yaml, there are already similary proptery, such as
> >>>>>>>>>> snps,incr-burst-type-adjustment. Use this method can keep whole dwc3 usb
> >>>>>>>>>> driver keep consistent. And not all platform try enable hardware
> >>>>>>>>>> dma_cohenrence. It is configable for difference platform.
> >>>>>>>>>
> >>>>>>>>> When you say "platform", what do you mean? I understand that term to
> >>>>>>>>> mean a combination of board, soc and firmware.
> >>>>>>>>
> >>>>>>>> In my company's environment, "platform" is "board". I will use "board" in
> >>>>>>>> future. Is it big difference here?
> >>>>>>>
> >>>>>>> Nah, that's close enough that it makes no difference here.
> >>>>>>>
> >>>>>>> I'd still like an explanation for why a platform would need to actually
> >>>>>>> set these properties though, and why information about coherency cannot
> >>>>>>> be determined from whether or not the boss the usb controller is on is
> >>>>>>> communicated to be dma coherent via the existing devicetree properties
> >>>>>>> for that purpose.
> >>>>>>
> >>>>>> Actually, I am not very clear about reason. I guest maybe treat off power
> >>>>>> consumption and performance.
> >>>>>>
> >>>>>> What's your judgement about proptery, which should be in dts. Such as
> >>>>>> reg, clk, reset, dma and irq, which is tighted with SOC. It is the fixed
> >>>>>> value for every SOC. The board dts never change these.
> >>>>>
> >>>>> Then it can be deduced from the compatible and there is no need for new
> >>>>> properties.
> >>>>
> >>>> Okay, I think "*reqinfo" match this. When new Soc(using compatible dwc usb
> >>>> controller) appear regardless dma-cohorence or not, connect by AXI3 or
> >>>> AXI4, needn't add new propterties. 
> >>>
> >>> Anyone have objection? I will prepare v2 to fix rob's bot error.
> >>
> >> I'm not sure what you want me to object to/not object to.
> >> Your last message said "needn't add new propterties", seemingly in
> >> agreement with Krzysztoff saying that it can be deduced from the
> >> compatible. That seems like a good way forward for me.
> > 
> > Okay, let me clear it again. dwc usb is quite common IP. The below is
> > what reason why need "*reginfo* instead of using compatible string.
> > 
> > 1. *reginfo* property is decscript hardware behevior, which will be changed
> > at difference SOC.
> > 2. it may change at board level according to if enable dma coherence.
> 
> dma coherence is not a board property. Anyway, you said it will never
> change in the board.

Sorry, let's correct what my previous said. There are two kinds bus in
system, one is dma_coherence, the other is none dma_coherence. There are
some dwc usb core ip, which is the exact the same. Some connect to
dma_coherence bus, some connect to non-dma_coherence bus.

So dma_coherence will be varible for this case. we need set *reginfo* for
dwc usb core, which connnect to dma_coherence bus.  not set "reginfo* for
the dwc usb core, which connect to none dma_coherence bus.

If use difference compatible string, that's means using difference
compatible string to descript one same hardware. You reject this method at
my svc-i3c-master\svc-i3c-target patches.

Frank

> 
> > 3. dwc core part is quite common, all SOC using common "snps, dwc3" as
> > core-part, all soc specific "nxp, dwc3 *", "qcom, dwc3*" is used for glue
> > logic part.
> 
> And all should be having dedicated compatibles.
> 
> > 4. using *reginfo* can reduce add more strange compatible string such as
> > "nxp, dwc3-core" ...
> > 5. *reginfo* property likes "reg", "clk", and align what Kryzystoff said.
> > "reg", "clk" is fixed for specfic SOC. These can help reduce "compatible"
> > string number. "reginfo" do the same work as "reg", "clk" ..
> 
> So again, reginfo is fixed for specific SoC? So it can be deduced from
> compatible.
> 
> I don't know what to say more here... so let's be clear that you
> understood me:
> 
> NAK
> 
> Best regards,
> Krzysztof
>