Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add system bus request info

[Frank Li]

On Tue, Jan 30, 2024 at 05:10:54PM +0100, Krzysztof Kozlowski wrote:
> On 30/01/2024 16:20, Frank Li wrote:
> > 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.
> 
> OK, that makes sense. Please provide link to upstream DTS
> (mainline/next/lore link/other upstream projects) showing this.

Look like I still have not found the real case yet. But I think it is
existed (maybe not upstream yet). Can I forward using this method?

Frank

> 
> Best regards,
> Krzysztof
>