| Message ID | 20221207162435.1001782-4-herve.codina@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add the Renesas USBF controller support | expand |
On 07/12/2022 17:24, Herve Codina wrote: > The 'depends-on' property is set in involved DTS. > > Move it to a required property. > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > --- > Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + This should be squashed with previous patch. There is no point to add property and immediately in the next patch make it required. Remember that bindings are separate from DTS. Best regards, Krzysztof
Hi Krzysztof, On Thu, 8 Dec 2022 09:26:41 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 07/12/2022 17:24, Herve Codina wrote: > > The 'depends-on' property is set in involved DTS. > > > > Move it to a required property. > > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > > --- > > Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + > > This should be squashed with previous patch. There is no point to add > property and immediately in the next patch make it required. Remember > that bindings are separate from DTS. > > Best regards, > Krzysztof > I though about make dtbs_check in case of git bisect. But, ok I will squash or perhaps remove completely this commit. It introduces a DT compatibility break adding a new mandatory property (raised by Rob on cover letter review). Is this compatibility break can be acceptable ? Best regards, Herve
On 08/12/2022 10:05, Herve Codina wrote: > Hi Krzysztof, > > On Thu, 8 Dec 2022 09:26:41 +0100 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 07/12/2022 17:24, Herve Codina wrote: >>> The 'depends-on' property is set in involved DTS. >>> >>> Move it to a required property. >>> >>> Signed-off-by: Herve Codina <herve.codina@bootlin.com> >>> --- >>> Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + >> >> This should be squashed with previous patch. There is no point to add >> property and immediately in the next patch make it required. Remember >> that bindings are separate from DTS. >> >> Best regards, >> Krzysztof >> > > I though about make dtbs_check in case of git bisect. And what would this commit change? In Git you will have 1. dt-bindings: PCI: renesas,pci-rcar-gen2: Add depends-on for RZ/N1 SoC family 2. dt-bindings: PCI: renesas,pci-rcar-gen2: 'depends-on' is no more optional so what is the difference for git bisect? > > But, ok I will squash or perhaps remove completely this commit. > It introduces a DT compatibility break adding a new mandatory > property (raised by Rob on cover letter review). > Is this compatibility break can be acceptable ? Requiring property in bindings as a fix for something which was broken is ok. But this is independent of Linux drivers, which should not stop working. Best regards, Krzysztof
Hi Krzysztof, On Thu, 8 Dec 2022 10:46:32 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 08/12/2022 10:05, Herve Codina wrote: > > Hi Krzysztof, > > > > On Thu, 8 Dec 2022 09:26:41 +0100 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > >> On 07/12/2022 17:24, Herve Codina wrote: > >>> The 'depends-on' property is set in involved DTS. > >>> > >>> Move it to a required property. > >>> > >>> Signed-off-by: Herve Codina <herve.codina@bootlin.com> > >>> --- > >>> Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + > >> > >> This should be squashed with previous patch. There is no point to add > >> property and immediately in the next patch make it required. Remember > >> that bindings are separate from DTS. > >> > >> Best regards, > >> Krzysztof > >> > > > > I though about make dtbs_check in case of git bisect. > > And what would this commit change? In Git you will have > 1. dt-bindings: PCI: renesas,pci-rcar-gen2: Add depends-on for RZ/N1 SoC > family > 2. dt-bindings: PCI: renesas,pci-rcar-gen2: 'depends-on' is no more optional > > so what is the difference for git bisect? Well, today, I have: 1. dt-bindings: Add depends-on 2. dts: Add depends-on 3. dt-bindings: Move depends-on to mandatory If I squash dt-bindings commits, I am going to have: 1. dt-bindings: Add mandatory depends-on 2. dts: Add depends-on or 1. dts: Add depends-on 2. dt-bindings: Add mandatory depends-on I have not tested but if I used only the first commit in each case (git bisect): In the first case, dtbs_check is probably going to signal the missing 'depends-on' property on dts. In the second case, dtbs_check is probably going to signal the not described 'depends-on' property present in dts. > > > > > But, ok I will squash or perhaps remove completely this commit. > > It introduces a DT compatibility break adding a new mandatory > > property (raised by Rob on cover letter review). > > Is this compatibility break can be acceptable ? > > Requiring property in bindings as a fix for something which was broken > is ok. But this is independent of Linux drivers, which should not stop > working. Ok, thanks. > > Best regards, > Krzysztof > Regards, Hervé
On 08/12/2022 16:51, Herve Codina wrote: > Hi Krzysztof, > > On Thu, 8 Dec 2022 10:46:32 +0100 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 08/12/2022 10:05, Herve Codina wrote: >>> Hi Krzysztof, >>> >>> On Thu, 8 Dec 2022 09:26:41 +0100 >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >>> >>>> On 07/12/2022 17:24, Herve Codina wrote: >>>>> The 'depends-on' property is set in involved DTS. >>>>> >>>>> Move it to a required property. >>>>> >>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com> >>>>> --- >>>>> Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + >>>> >>>> This should be squashed with previous patch. There is no point to add >>>> property and immediately in the next patch make it required. Remember >>>> that bindings are separate from DTS. >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >>> I though about make dtbs_check in case of git bisect. >> >> And what would this commit change? In Git you will have >> 1. dt-bindings: PCI: renesas,pci-rcar-gen2: Add depends-on for RZ/N1 SoC >> family >> 2. dt-bindings: PCI: renesas,pci-rcar-gen2: 'depends-on' is no more optional >> >> so what is the difference for git bisect? > > Well, today, I have: > 1. dt-bindings: Add depends-on > 2. dts: Add depends-on > 3. dt-bindings: Move depends-on to mandatory What does it mean "I have"? Patches on mailing list? But we talk about Git and I wrote you bindings are DTS are not going the same tree. > > If I squash dt-bindings commits, I am going to have: > 1. dt-bindings: Add mandatory depends-on > 2. dts: Add depends-on > or > 1. dts: Add depends-on > 2. dt-bindings: Add mandatory depends-on And how does it matter? Anyway it goes separate trees. > > I have not tested but if I used only the first commit in each > case (git bisect): It's not bisectable anyway, you cannot make it bisectable within one release. > In the first case, dtbs_check is probably going to signal the > missing 'depends-on' property on dts. > In the second case, dtbs_check is probably going to signal the > not described 'depends-on' property present in dts. And why is that even a problem? Best regards, Krzysztof
Hi Krzysztof, On Fri, 9 Dec 2022 09:06:55 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 08/12/2022 16:51, Herve Codina wrote: > > Hi Krzysztof, > > > > On Thu, 8 Dec 2022 10:46:32 +0100 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > >> On 08/12/2022 10:05, Herve Codina wrote: > >>> Hi Krzysztof, > >>> > >>> On Thu, 8 Dec 2022 09:26:41 +0100 > >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >>> > >>>> On 07/12/2022 17:24, Herve Codina wrote: > >>>>> The 'depends-on' property is set in involved DTS. > >>>>> > >>>>> Move it to a required property. > >>>>> > >>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com> > >>>>> --- > >>>>> Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + > >>>> > >>>> This should be squashed with previous patch. There is no point to add > >>>> property and immediately in the next patch make it required. Remember > >>>> that bindings are separate from DTS. > >>>> > >>>> Best regards, > >>>> Krzysztof > >>>> > >>> > >>> I though about make dtbs_check in case of git bisect. > >> > >> And what would this commit change? In Git you will have > >> 1. dt-bindings: PCI: renesas,pci-rcar-gen2: Add depends-on for RZ/N1 SoC > >> family > >> 2. dt-bindings: PCI: renesas,pci-rcar-gen2: 'depends-on' is no more optional > >> > >> so what is the difference for git bisect? > > > > Well, today, I have: > > 1. dt-bindings: Add depends-on > > 2. dts: Add depends-on > > 3. dt-bindings: Move depends-on to mandatory > > What does it mean "I have"? Patches on mailing list? But we talk about > Git and I wrote you bindings are DTS are not going the same tree. > > > > > If I squash dt-bindings commits, I am going to have: > > 1. dt-bindings: Add mandatory depends-on > > 2. dts: Add depends-on > > or > > 1. dts: Add depends-on > > 2. dt-bindings: Add mandatory depends-on > > And how does it matter? Anyway it goes separate trees. I finally understand what you mean by separate trees. And indeed, you're right, my patches split does not make any sense. According to feedbacks on this v3 series, these 3 patches will be removed in v4. Thanks for the review, Hervé
diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml index e1221ad68465..4dd0f2f72e62 100644 --- a/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml +++ b/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml @@ -138,6 +138,7 @@ allOf: the underlying USB hosts start. required: - clock-names + - depends-on else: properties: clocks:
The 'depends-on' property is set in involved DTS. Move it to a required property. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 + 1 file changed, 1 insertion(+)