[PATCH v2 1/2] dt-bindings: fpga: Also describe clock for gpio

[PATCH v2 1/2] dt-bindings: fpga: Also describe clock for gpio

[Michal Simek]

Axi gpio is going to have clocks as required property that's why it should
be also described in bindings which are using axi gpio node.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v2:
- New patch to fix reported as issue by the second patch
- https://lore.kernel.org/r/174954437576.4177094.15371626866789542129.robh@kernel.org

 Documentation/devicetree/bindings/fpga/fpga-region.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.yaml b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
index 77554885a6c4..7d2d3b7aa4b7 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.yaml
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
@@ -316,6 +316,7 @@ examples:
         reg = <0x40000000 0x10000>;
         gpio-controller;
         #gpio-cells = <2>;
+        clocks = <&clk>;
       };
     };
 
-- 
2.43.0

[PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Michal Simek]

On Microblaze platforms there is no need to handle clocks because the
system is starting with clocks enabled (can be described via fixed clock
node or clock-frequency property or not described at all).
With using soft IPs with SOC platforms there is mandatory to handle clocks
as is explained in commit 60dbdc6e08d6 ("dt-bindings: net: emaclite: Add
clock support").
That's why make clock as required in dt binding because it is present in
both configurations and should be described even there is no way how to
handle it on Microblaze systems.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v2:
- Add fpga region patch to fix issue which was introduces by this change

Based on discussion at
https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/

Actually this shouldn't be only targetting GPIO but also for example
xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
before starting to check other bindings.

---
 Documentation/devicetree/bindings/gpio/xlnx,gpio-xilinx.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/xlnx,gpio-xilinx.yaml b/Documentation/devicetree/bindings/gpio/xlnx,gpio-xilinx.yaml
index 8fbf12ca067e..7af4eb2d1858 100644
--- a/Documentation/devicetree/bindings/gpio/xlnx,gpio-xilinx.yaml
+++ b/Documentation/devicetree/bindings/gpio/xlnx,gpio-xilinx.yaml
@@ -117,6 +117,7 @@ properties:
 required:
   - reg
   - compatible
+  - clocks
   - gpio-controller
   - "#gpio-cells"
 
-- 
2.43.0

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Krzysztof Kozlowski]

On 13/06/2025 12:12, Michal Simek wrote:
> On Microblaze platforms there is no need to handle clocks because the
> system is starting with clocks enabled (can be described via fixed clock
> node or clock-frequency property or not described at all).
> With using soft IPs with SOC platforms there is mandatory to handle clocks
> as is explained in commit 60dbdc6e08d6 ("dt-bindings: net: emaclite: Add
> clock support").
> That's why make clock as required in dt binding because it is present in
> both configurations and should be described even there is no way how to
> handle it on Microblaze systems.
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
> Changes in v2:
> - Add fpga region patch to fix issue which was introduces by this change
> 
> Based on discussion at
> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
> 
> Actually this shouldn't be only targetting GPIO but also for example
> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
> before starting to check other bindings.

IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
dt_binding_check is not bisectable and we want it to be bisectable.

(dtbs_check does not have to be, as long as linux-next is concerned)

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Michal Simek]

On 6/13/25 13:07, Krzysztof Kozlowski wrote:
> On 13/06/2025 12:12, Michal Simek wrote:
>> On Microblaze platforms there is no need to handle clocks because the
>> system is starting with clocks enabled (can be described via fixed clock
>> node or clock-frequency property or not described at all).
>> With using soft IPs with SOC platforms there is mandatory to handle clocks
>> as is explained in commit 60dbdc6e08d6 ("dt-bindings: net: emaclite: Add
>> clock support").
>> That's why make clock as required in dt binding because it is present in
>> both configurations and should be described even there is no way how to
>> handle it on Microblaze systems.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Changes in v2:
>> - Add fpga region patch to fix issue which was introduces by this change
>>
>> Based on discussion at
>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>
>> Actually this shouldn't be only targetting GPIO but also for example
>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>> before starting to check other bindings.
> 
> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
> dt_binding_check is not bisectable and we want it to be bisectable.

No issue with squash if necessary. I sent it as series to be applied together 
which won't break bisectability of tree and no new error is going to be reported.

Thanks,
Michal

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Krzysztof Kozlowski]

On 13/06/2025 13:26, Michal Simek wrote:
>>> Based on discussion at
>>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>>
>>> Actually this shouldn't be only targetting GPIO but also for example
>>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>>> before starting to check other bindings.
>>
>> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
>> dt_binding_check is not bisectable and we want it to be bisectable.
> 
> No issue with squash if necessary. I sent it as series to be applied together 
> which won't break bisectability of tree and no new error is going to be reported.

You did not say anything about dependencies and merging strategy, to
this would go via different trees. Sending something in one patchset
does not mean that there is a dependency.

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Michal Simek]

Hi,

On 6/16/25 08:41, Krzysztof Kozlowski wrote:
> On 13/06/2025 13:26, Michal Simek wrote:
>>>> Based on discussion at
>>>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>>>
>>>> Actually this shouldn't be only targetting GPIO but also for example
>>>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>>>> before starting to check other bindings.
>>>
>>> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
>>> dt_binding_check is not bisectable and we want it to be bisectable.
>>
>> No issue with squash if necessary. I sent it as series to be applied together
>> which won't break bisectability of tree and no new error is going to be reported.
> 
> You did not say anything about dependencies and merging strategy, to
> this would go via different trees. Sending something in one patchset
> does not mean that there is a dependency.

No offense but I don't think I can agree with this. The main purpose of patchset 
is to show sequence how things should go one after each other and series should 
go via single tree.
Sometimes people are asking for picking up individual patches from series but 
this is not a normal way. Also seen a lot of time asking for splitting that 
patches and send them individually instead of picking up from the series.

Anyway. If you want to me to squash it together I am fine with it.
If you want to me to create cover letter that's fine for me too.

Thanks,
Michal

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Krzysztof Kozlowski]

On 16/06/2025 08:51, Michal Simek wrote:
> Hi,
> 
> On 6/16/25 08:41, Krzysztof Kozlowski wrote:
>> On 13/06/2025 13:26, Michal Simek wrote:
>>>>> Based on discussion at
>>>>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>>>>
>>>>> Actually this shouldn't be only targetting GPIO but also for example
>>>>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>>>>> before starting to check other bindings.
>>>>
>>>> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
>>>> dt_binding_check is not bisectable and we want it to be bisectable.
>>>
>>> No issue with squash if necessary. I sent it as series to be applied together
>>> which won't break bisectability of tree and no new error is going to be reported.
>>
>> You did not say anything about dependencies and merging strategy, to
>> this would go via different trees. Sending something in one patchset
>> does not mean that there is a dependency.
> 
> No offense but I don't think I can agree with this. The main purpose of patchset 
> is to show sequence how things should go one after each other and series should 
> go via single tree.

Go through all patchsets on DT list touching different subsystems. You
will find only 1% of patchsets having above expectation implied (when
not explicitly stated).

Really. 99% of patchsets on DT list targeting different subsytems, have
opposite, so implied rule they go INDEPENDENTLY to separate subsystems.

And above (so implied rule of splitting things) is even documented in DT
submitting patches.

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Krzysztof Kozlowski]

On 16/06/2025 09:10, Krzysztof Kozlowski wrote:
> On 16/06/2025 08:51, Michal Simek wrote:
>> Hi,
>>
>> On 6/16/25 08:41, Krzysztof Kozlowski wrote:
>>> On 13/06/2025 13:26, Michal Simek wrote:
>>>>>> Based on discussion at
>>>>>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>>>>>
>>>>>> Actually this shouldn't be only targetting GPIO but also for example
>>>>>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>>>>>> before starting to check other bindings.
>>>>>
>>>>> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
>>>>> dt_binding_check is not bisectable and we want it to be bisectable.
>>>>
>>>> No issue with squash if necessary. I sent it as series to be applied together
>>>> which won't break bisectability of tree and no new error is going to be reported.
>>>
>>> You did not say anything about dependencies and merging strategy, to
>>> this would go via different trees. Sending something in one patchset
>>> does not mean that there is a dependency.
>>
>> No offense but I don't think I can agree with this. The main purpose of patchset 
>> is to show sequence how things should go one after each other and series should 
>> go via single tree.
> 
> Go through all patchsets on DT list touching different subsystems. You
> will find only 1% of patchsets having above expectation implied (when
> not explicitly stated).
> 
> Really. 99% of patchsets on DT list targeting different subsytems, have
> opposite, so implied rule they go INDEPENDENTLY to separate subsystems.
> 
> And above (so implied rule of splitting things) is even documented in DT
> submitting patches.
> 
One more thought: That was from submitter point of view. But from
maintainers point of view, EVERY MONTH there is around one patchset on
DT list which has implied merging like you described (but not explicitly
stated) and MAINTAINERS pick them up independently causing breaks, so
some or many MAINTAINERS also have such reasoning as I said.

They will pick up individual bits from patchset unless told otherwise.

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Michal Simek]

On 6/16/25 09:13, Krzysztof Kozlowski wrote:
> On 16/06/2025 09:10, Krzysztof Kozlowski wrote:
>> On 16/06/2025 08:51, Michal Simek wrote:
>>> Hi,
>>>
>>> On 6/16/25 08:41, Krzysztof Kozlowski wrote:
>>>> On 13/06/2025 13:26, Michal Simek wrote:
>>>>>>> Based on discussion at
>>>>>>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>>>>>>
>>>>>>> Actually this shouldn't be only targetting GPIO but also for example
>>>>>>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>>>>>>> before starting to check other bindings.
>>>>>>
>>>>>> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
>>>>>> dt_binding_check is not bisectable and we want it to be bisectable.
>>>>>
>>>>> No issue with squash if necessary. I sent it as series to be applied together
>>>>> which won't break bisectability of tree and no new error is going to be reported.
>>>>
>>>> You did not say anything about dependencies and merging strategy, to
>>>> this would go via different trees. Sending something in one patchset
>>>> does not mean that there is a dependency.
>>>
>>> No offense but I don't think I can agree with this. The main purpose of patchset
>>> is to show sequence how things should go one after each other and series should
>>> go via single tree.
>>
>> Go through all patchsets on DT list touching different subsystems. You
>> will find only 1% of patchsets having above expectation implied (when
>> not explicitly stated).
>>
>> Really. 99% of patchsets on DT list targeting different subsytems, have
>> opposite, so implied rule they go INDEPENDENTLY to separate subsystems.
>>
>> And above (so implied rule of splitting things) is even documented in DT
>> submitting patches.
>>
> One more thought: That was from submitter point of view. But from
> maintainers point of view, EVERY MONTH there is around one patchset on
> DT list which has implied merging like you described (but not explicitly
> stated) and MAINTAINERS pick them up independently causing breaks, so
> some or many MAINTAINERS also have such reasoning as I said.
> 
> They will pick up individual bits from patchset unless told otherwise.

What do you want me to do?

Thanks,
Michal

Re: [PATCH v2 2/2] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property

[Krzysztof Kozlowski]

On 16/06/2025 09:18, Michal Simek wrote:
> 
> 
> On 6/16/25 09:13, Krzysztof Kozlowski wrote:
>> On 16/06/2025 09:10, Krzysztof Kozlowski wrote:
>>> On 16/06/2025 08:51, Michal Simek wrote:
>>>> Hi,
>>>>
>>>> On 6/16/25 08:41, Krzysztof Kozlowski wrote:
>>>>> On 13/06/2025 13:26, Michal Simek wrote:
>>>>>>>> Based on discussion at
>>>>>>>> https://lore.kernel.org/lkml/20241002-revivable-crummy-f780adec538c@spud/
>>>>>>>>
>>>>>>>> Actually this shouldn't be only targetting GPIO but also for example
>>>>>>>> xlnx,xps-timebase-wdt-1.00.a but I would like to check it first on gpio
>>>>>>>> before starting to check other bindings.
>>>>>>>
>>>>>>> IIUC, patch #1 is a prerequisite, so you need to squash them. Otherwise
>>>>>>> dt_binding_check is not bisectable and we want it to be bisectable.
>>>>>>
>>>>>> No issue with squash if necessary. I sent it as series to be applied together
>>>>>> which won't break bisectability of tree and no new error is going to be reported.
>>>>>
>>>>> You did not say anything about dependencies and merging strategy, to
>>>>> this would go via different trees. Sending something in one patchset
>>>>> does not mean that there is a dependency.
>>>>
>>>> No offense but I don't think I can agree with this. The main purpose of patchset
>>>> is to show sequence how things should go one after each other and series should
>>>> go via single tree.
>>>
>>> Go through all patchsets on DT list touching different subsystems. You
>>> will find only 1% of patchsets having above expectation implied (when
>>> not explicitly stated).
>>>
>>> Really. 99% of patchsets on DT list targeting different subsytems, have
>>> opposite, so implied rule they go INDEPENDENTLY to separate subsystems.
>>>
>>> And above (so implied rule of splitting things) is even documented in DT
>>> submitting patches.
>>>
>> One more thought: That was from submitter point of view. But from
>> maintainers point of view, EVERY MONTH there is around one patchset on
>> DT list which has implied merging like you described (but not explicitly
>> stated) and MAINTAINERS pick them up independently causing breaks, so
>> some or many MAINTAINERS also have such reasoning as I said.
>>
>> They will pick up individual bits from patchset unless told otherwise.
> 
> What do you want me to do?
We just discuss about the process. You disagreed with me, I responded.
If you ask in general how to solve such problems: either squash such
patches or document the dependency/merging strategy.

General kernel submitting patches also asks about this:

https://elixir.bootlin.com/linux/v6.16-rc1/source/Documentation/process/submitting-patches.rst#L186

(which is third argument against your implied dependency within patchset).

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: fpga: Also describe clock for gpio

[Xu Yilun]

On Fri, Jun 13, 2025 at 12:12:52PM +0200, Michal Simek wrote:
> Axi gpio is going to have clocks as required property that's why it should
> be also described in bindings which are using axi gpio node.
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
> Changes in v2:
> - New patch to fix reported as issue by the second patch
> - https://lore.kernel.org/r/174954437576.4177094.15371626866789542129.robh@kernel.org
> 
>  Documentation/devicetree/bindings/fpga/fpga-region.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.yaml b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
> index 77554885a6c4..7d2d3b7aa4b7 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.yaml
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
> @@ -316,6 +316,7 @@ examples:
>          reg = <0x40000000 0x10000>;
>          gpio-controller;
>          #gpio-cells = <2>;
> +        clocks = <&clk>;

This file is mainly for fpga-region bindings. So I don't think we have
to strictly align with the example IP block binding every time it has
an update.

Thanks,
Yilun

>        };
>      };
>  
> -- 
> 2.43.0
> 
> 

Re: [PATCH v2 1/2] dt-bindings: fpga: Also describe clock for gpio

[Michal Simek]

On 6/13/25 12:44, Xu Yilun wrote:
> On Fri, Jun 13, 2025 at 12:12:52PM +0200, Michal Simek wrote:
>> Axi gpio is going to have clocks as required property that's why it should
>> be also described in bindings which are using axi gpio node.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Changes in v2:
>> - New patch to fix reported as issue by the second patch
>> - https://lore.kernel.org/r/174954437576.4177094.15371626866789542129.robh@kernel.org
>>
>>   Documentation/devicetree/bindings/fpga/fpga-region.yaml | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.yaml b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
>> index 77554885a6c4..7d2d3b7aa4b7 100644
>> --- a/Documentation/devicetree/bindings/fpga/fpga-region.yaml
>> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
>> @@ -316,6 +316,7 @@ examples:
>>           reg = <0x40000000 0x10000>;
>>           gpio-controller;
>>           #gpio-cells = <2>;
>> +        clocks = <&clk>;
> 
> This file is mainly for fpga-region bindings. So I don't think we have
> to strictly align with the example IP block binding every time it has
> an update.

But Rob's script are reporting issue if they are not. Please take a look at link 
above.

M

Re: [PATCH v2 1/2] dt-bindings: fpga: Also describe clock for gpio

[Xu Yilun]

On Fri, Jun 13, 2025 at 12:52:46PM +0200, Michal Simek wrote:
> 
> 
> On 6/13/25 12:44, Xu Yilun wrote:
> > On Fri, Jun 13, 2025 at 12:12:52PM +0200, Michal Simek wrote:
> > > Axi gpio is going to have clocks as required property that's why it should
> > > be also described in bindings which are using axi gpio node.
> > > 
> > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > ---
> > > 
> > > Changes in v2:
> > > - New patch to fix reported as issue by the second patch
> > > - https://lore.kernel.org/r/174954437576.4177094.15371626866789542129.robh@kernel.org
> > > 
> > >   Documentation/devicetree/bindings/fpga/fpga-region.yaml | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.yaml b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
> > > index 77554885a6c4..7d2d3b7aa4b7 100644
> > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.yaml
> > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.yaml
> > > @@ -316,6 +316,7 @@ examples:
> > >           reg = <0x40000000 0x10000>;
> > >           gpio-controller;
> > >           #gpio-cells = <2>;
> > > +        clocks = <&clk>;
> > 
> > This file is mainly for fpga-region bindings. So I don't think we have
> > to strictly align with the example IP block binding every time it has
> > an update.
> 
> But Rob's script are reporting issue if they are not. Please take a look at
> link above.

I see, then from FPGA side

Reviewed-by: Xu Yilun <yilun.xu@intel.com>
> 
> M