The 'depends-on' property is set in involved DTS.
Move it to a required property.
Signed-off-by: Herve Codina <[email protected]>
---
Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml | 1 +
1 file changed, 1 insertion(+)
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:
--
2.38.1
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 <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]> 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 <[email protected]>
>>> ---
>>> 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 <[email protected]> wrote:
> On 08/12/2022 10:05, Herve Codina wrote:
> > Hi Krzysztof,
> >
> > On Thu, 8 Dec 2022 09:26:41 +0100
> > Krzysztof Kozlowski <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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 <[email protected]> wrote:
>
>> On 08/12/2022 10:05, Herve Codina wrote:
>>> Hi Krzysztof,
>>>
>>> On Thu, 8 Dec 2022 09:26:41 +0100
>>> Krzysztof Kozlowski <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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 <[email protected]> wrote:
> On 08/12/2022 16:51, Herve Codina wrote:
> > Hi Krzysztof,
> >
> > On Thu, 8 Dec 2022 10:46:32 +0100
> > Krzysztof Kozlowski <[email protected]> wrote:
> >
> >> On 08/12/2022 10:05, Herve Codina wrote:
> >>> Hi Krzysztof,
> >>>
> >>> On Thu, 8 Dec 2022 09:26:41 +0100
> >>> Krzysztof Kozlowski <[email protected]> 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 <[email protected]>
> >>>>> ---
> >>>>> 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é
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com