On 08/06/2023 13:34, Michal Simek wrote: > > > On 6/8/23 13:29, Krzysztof Kozlowski wrote: >> On 08/06/2023 12:55, Mehta, Piyush wrote: >>> Hello, >>> >>>> -----Original Message----- >>>> From: Rob Herring <robh@xxxxxxxxxx> >>>> Sent: Wednesday, June 7, 2023 1:06 PM >>>> To: Mehta, Piyush <piyush.mehta@xxxxxxx> >>>> Cc: p.zabel@xxxxxxxxxxxxxx; Paladugu, Siva Durga Prasad >>>> <siva.durga.prasad.paladugu@xxxxxxx>; michal.simek@xxxxxxxxxx; >>>> nava.manne@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- >>>> kernel@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; conor+dt@xxxxxxxxxx; >>>> krzysztof.kozlowski+dt@xxxxxxxxxx; robh+dt@xxxxxxxxxx; Simek, Michal >>>> <michal.simek@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx> >>>> Subject: Re: [PATCH 1/2] dt-bindings: reset: convert the xlnx,zynqmp-reset.txt >>>> to yaml >>>> >>>> >>>> On Wed, 07 Jun 2023 12:21:20 +0530, Piyush Mehta wrote: >>>>> Convert the binding to DT schema format. It also updates the >>>>> reset-controller description. >>>>> >>>>> Signed-off-by: Piyush Mehta <piyush.mehta@xxxxxxx> >>>>> --- >>>>> Note: >>>>> The 1/2 patch introduced a warning related to the dt binding check, >>>>> which was subsequently fixed in the 2/2 patch. This warning is a >>>>> conversion warning due to a dependency on firmware dt binding. >>>>> >>>>> Warning: >>>>> DTC_CHK >>>>> Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.example.dtb >>>>> >>>> /scratch/piyushm/torvalds/linux/Documentation/devicetree/bindings/reset/xln >>>> x,zynqmp-reset.example.dtb: zynqmp-firmware: 'reset-controller' does not >>>> match any of the regexes: 'pinctrl-[0-9]+' >>>>> From schema: >>>>> /scratch/piyushm/torvalds/linux/Documentation/devicetree/bindings/firm >>>>> ware/xilinx/xlnx,zynqmp-firmware.yaml >>>>> >>>>> Please suggest to me, if it's fine to make a single patch instead of >>>>> the separate patch 1/2 and 2/2. >>>>> --- >>>>> .../bindings/reset/xlnx,zynqmp-reset.txt | 55 --------------- >>>>> .../bindings/reset/xlnx,zynqmp-reset.yaml | 67 +++++++++++++++++++ >>>>> 2 files changed, 67 insertions(+), 55 deletions(-) delete mode >>>>> 100644 Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.yaml >>>>> >>>> >>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >>>> on your patch (DT_CHECKER_FLAGS is new in v5.13): >>>> >>>> yamllint warnings/errors: >>>> >>>> dtschema/dtc warnings/errors: >>>> /builds/robherring/dt-review- >>>> ci/linux/Documentation/devicetree/bindings/reset/xlnx,zynqmp- >>>> reset.example.dtb: zynqmp-firmware: 'reset-controller' does not match any of >>>> the regexes: 'pinctrl-[0-9]+' >>>> From schema: /builds/robherring/dt-review- >>>> ci/linux/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- >>>> firmware.yaml >>>> >>>> doc reference errors (make refcheckdocs): >>>> >>>> See https://patchwork.ozlabs.org/project/devicetree- >>>> bindings/patch/20230607065121.2265076-2-piyush.mehta@xxxxxxx >>>> >>>> The base for the series is generally the latest rc1. A different dependency >>>> should be noted in *this* patch. >>>> >>>> If you already ran 'make dt_binding_check' and didn't see the above error(s), >>>> then make sure 'yamllint' is installed and dt-schema is up to >>>> date: >>>> >>>> pip3 install dtschema --upgrade >>>> >>>> Please check and re-submit after running the above command yourself. Note >>>> that DT_SCHEMA_FILES can be set to your schema file to speed up checking >>>> your schema. However, it must be unset to test all examples with your schema. >>> >>> This dt_binding warning is known to us and is captured in commit description notes and to resolve it, we need to apply 2/2. >> >> So your patchset has broken bisectability. You need to fix it. If the >> warning was known, then I am surprised that you sent broken patchset. > > The first patch is conversion from txt yaml without any additional changes > because in past we heart that we shouldn't really do other things in it. That's > why 2 patches where made. No really issue to squashed them together. We always shared feedback that conversion should do only conversion, but a correct one. Conversion cannot introduce errors or warnings in the bindings. It can introduce warnings in DTS, but not in the bindings. This was also communicated several times. If pure-conversion results in non-bisectable patchset then: 1. either conversion should come with changes, 2. or conversion is done a bit wrong or could be done differently like here. You really do not need unrelated pieces of DTS in reset controller code. Drop them. Best regards, Krzysztof
