> -----Original Message----- > From: Felipe Balbi [mailto:balbi@xxxxxxxxxx] > Sent: Tuesday, January 17, 2017 6:45 PM > To: Jerry Huang <jerry.huang@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx> > Cc: mark.rutland@xxxxxxx; catalin.marinas@xxxxxxx; > will.deacon@xxxxxxx; linux@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > linux-usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx > Subject: RE: [PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst- > type-adjustment" for INCR burst type > > > Hi, > > Jerry Huang <jerry.huang@xxxxxxx> writes: > > <snip> > > >> >> So, I think we still need two vaue to specify INCRBrstEna and > >> >> INCRx burst type. > >> > Hi, Balbi, > >> > It seems there is no feedback for my comment, so these patches can > >> > be > >> accepted? > >> > >> probably not, we need to really understand what information we need > >> so it can be described properly. The last thing we want is > >> unnecessary DT properties. > >> > >> It seems to me that we can extrapolate INCRBrstEna based on which > >> burst modes are enabled. If only 0 is passed, then that bit should be > >> 1, if 0 and any other size is passed, then that bit should be 0, no? > > Hi, Balbi, > > Below is the definition for this property, > > snps,incr-burst-type-adjustment = <x>, <y> > > x: Undefined Length INCR Burst Type Enable (INCRBrstEna) > > 0 - INCRX burst mode (not enable INCRBrstEna) > > 1 - INCR (undefined length) burst mode (enable INCRBrstEna) > > y: the burst length > > > > 1> if x = 0: means INCRBrstEna not enabled, set bit0 to zero (or clear > > it) , we select one of > > INCR1/INCR4/INCR8/INCR16/INCR32/INCR64/INCR128/INCR256 (pass this > > value through "y")to set the fix burst length controller supported. > > > > For example: > > > > snps,incr-burst-type-adjustment = <0>, <16> > > > > driver will set bit0 to zero and set bit3 to 1 (INCR16 Burst Type > > Enabled), controller will use INCR16 (with 16 bytes) to transfer data. > > > > 2> if x = 1: means INCRBrstEna enabled, we select one of > > INCR4/INCR8/INCR16/INCR32/INCR64/INCR128/INCR256 (pass this value > > through "y") to set the burst length, and controller will use any > > length less than or equal to that we selected. > > > > For example: > > > > snps,incr-burst-type-adjustment = <1>, <32> > > > > driver will set bit0 to 1 and set bit4 to 1 (INCR32 Burst Type > > Enabled), controller will use any burst length less than (such as 4, > > 8, 16 byte) or equal to 32 byte to transfer data. > > > > Therefore, I think this two fileds are needed. Do you think about it? > > no, I don't think two values are needed, because first value can be > extrapolated from the second. Something like this: > > snps,incr-burst-type-adjustment = <4>, <8>, <16>, <32>; > > This is basically telling us that we can support anything in this list. So > INCRBrstEna should be set to 1. > > If DT, on the other hand, says: > > snps,incr-burst-type-adjustment = <32>; > > this means that we can only support INCR32, so INCRBrstEna should be > cleared to 0. Got it, I will try this mode. Thanks a lot, Balbi, -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html