On 12/10/2020 17:13, Serge Semin wrote: > On Mon, Oct 12, 2020 at 05:01:43PM +0200, Neil Armstrong wrote: >> Hi, >> >> On 12/10/2020 16:22, Serge Semin wrote: >>> On Mon, Oct 12, 2020 at 09:54:25AM +0200, Neil Armstrong wrote: >>>> Hi, >>>> >>>> On 11/10/2020 00:41, Serge Semin wrote: >>>>> An empty snps,quirk-frame-length-adjustment won't cause any change >>>>> performed by the driver. Moreover the DT schema validation will fail, >>>>> since it expects the property being assigned with some value. So just >>>>> discard the property declaration then from the example. >>>>> >>>>> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> >>>>> >>>>> --- >>>>> >>>>> Note the same problem is in the DT source file >>>>> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . >>>>> --- >>>>> .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml | 1 - >>>>> 1 file changed, 1 deletion(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>>>> index 5b04a7dfa018..88184d7e26cc 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>>>> +++ b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml >>>>> @@ -209,6 +209,5 @@ examples: >>>>> interrupts = <30>; >>>>> dr_mode = "host"; >>>>> snps,dis_u2_susphy_quirk; >>>>> - snps,quirk-frame-length-adjustment; >>>>> }; >>>>> }; >>>>> >>>> >>> >>>> Thanks for reporting this, actually the fladj must be 0x20 on this hw, >>>> but we do set this on the PHY side, so we can let the dwc3 side 0 here. >>> >>> I can convert this patch to initializing the "snps,quirk-frame-length-adjustment" >>> property with 0x20 value instead. Since most likely I'll have to send a v2/v3/etc >>> of this patchset, that modification won't be too much work to do. What do you think? >> > >> Yes, do this please, > > Ok. Shall I preserve your Acked-by tag in the new patch or you'd prefer to > review it first? Yes, preserve it, Thanks, Neil >> anyway it's only an example so it's ok. > > Actually examples are also validated by "make dt_binding_check". That's why I > had to fix the amlogic,meson-g12a-usb-ctrl example for at least so the new > snps,dwc3.yaml DT schema wouldn't break that full DT bindings > validation procedure.) > > -Sergey > >> >>> >>> Anyway please note, that I've fixed the improper property usage in the DT schema >>> example only. "snps,quirk-frame-length-adjustment" defined as boolean still >>> persists in the DTS file: arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . >>> So if you ever try to validate that dts file with "make dtbs_check" scenario, it >>> will fail. >> >> Yes, I'll push a fix to pass the dtbs_check when this is merged. >> >> Thanks, >> Neil >> >>> >>> -Sergey >>> >>>> >>>> Acked-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >>>> >>>> Neil >>>> >>