diff mbox series

[02/17] dt-bindings: arm: apple: apple,pmgr: Add t8112-pmgr compatible

Message ID 20230202-asahi-t8112-dt-v1-2-cb5442d1c229@jannau.net (mailing list archive)
State New, archived
Headers show
Series Device trees for Apple M2 (t8112) based devices | expand

Commit Message

Janne Grunau Feb. 12, 2023, 3:41 p.m. UTC
The block on Apple M2 SoCs is compatible with the existing driver so
just add its per-SoC compatible.

Signed-off-by: Janne Grunau <j@jannau.net>

---
This trivial dt-bindings update should be merged through the asahi-soc
tree to ensure validation of the Apple M2 (t8112) devicetrees in this
series.
---
 Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski Feb. 13, 2023, 11:10 a.m. UTC | #1
On 12/02/2023 16:41, Janne Grunau wrote:
> The block on Apple M2 SoCs is compatible with the existing driver so
> just add its per-SoC compatible.
> 
> Signed-off-by: Janne Grunau <j@jannau.net>
> 
> ---
> This trivial dt-bindings update should be merged through the asahi-soc
> tree to ensure validation of the Apple M2 (t8112) devicetrees in this
> series.

No, the bindings go via subsystem. Just because you want to validate
something is not really a reason - you can validate on next. Don't
create special rules for Asahi... or rather - why Asahi is special than
everyone else?

> ---
>  Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml | 1 +
>  1 file changed, 1 insertion(+)


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Janne Grunau Feb. 13, 2023, 11:57 a.m. UTC | #2
On 2023-02-13 12:10:36 +0100, Krzysztof Kozlowski wrote:
> On 12/02/2023 16:41, Janne Grunau wrote:
> > The block on Apple M2 SoCs is compatible with the existing driver so
> > just add its per-SoC compatible.
> > 
> > Signed-off-by: Janne Grunau <j@jannau.net>
> > 
> > ---
> > This trivial dt-bindings update should be merged through the asahi-soc
> > tree to ensure validation of the Apple M2 (t8112) devicetrees in this
> > series.
> 
> No, the bindings go via subsystem. Just because you want to validate
> something is not really a reason - you can validate on next. Don't
> create special rules for Asahi... or rather - why Asahi is special than
> everyone else?

We did that 2 or 3 times in the past without commnts that it is not 
desired so I wasn't aware that this would be special handling.

Merging binding and devicetree updates together looks to me like the 
most sensible option since dtbs validation is the only testable 
dependecy of dt binding updates.
Keeping them together ensures the dtbs validate without delaying 
devicetree changes by one kernel release after the dt-bindings change 
was merged.
I suppose it works out most of the time if the merge request is sent 
only if it validates in next. That still depends on the merge order in 
the merge window but -rc1 should be fine.

I'll consider devicetree validation as eventually valid from now on and 
not care too much about it.

Janne
Krzysztof Kozlowski Feb. 13, 2023, 12:10 p.m. UTC | #3
On 13/02/2023 12:57, Janne Grunau wrote:
> On 2023-02-13 12:10:36 +0100, Krzysztof Kozlowski wrote:
>> On 12/02/2023 16:41, Janne Grunau wrote:
>>> The block on Apple M2 SoCs is compatible with the existing driver so
>>> just add its per-SoC compatible.
>>>
>>> Signed-off-by: Janne Grunau <j@jannau.net>
>>>
>>> ---
>>> This trivial dt-bindings update should be merged through the asahi-soc
>>> tree to ensure validation of the Apple M2 (t8112) devicetrees in this
>>> series.
>>
>> No, the bindings go via subsystem. Just because you want to validate
>> something is not really a reason - you can validate on next. Don't
>> create special rules for Asahi... or rather - why Asahi is special than
>> everyone else?
> 
> We did that 2 or 3 times in the past without commnts that it is not 
> desired so I wasn't aware that this would be special handling.
> 
> Merging binding and devicetree updates together looks to me like the 
> most sensible option since dtbs validation is the only testable 
> dependecy of dt binding updates.

But it is not the recommended practice. Bindings were always going with
drivers and this was said by Rob multiple times.

For sure if there is no driver update at all or subsystem maintainer is
not responsive, bindings were picked up by SoC folks, but it's rather
fallback, not the main path.

> Keeping them together ensures the dtbs validate without delaying 
> devicetree changes by one kernel release after the dt-bindings change 
> was merged.

dtbs will validate on next and in next release the same way if bindings
go via subsystem. I don't see the benefit nor any difference for
validation. What type of delay? Why would you ever need it?

> I suppose it works out most of the time if the merge request is sent 
> only if it validates in next. That still depends on the merge order in 
> the merge window but -rc1 should be fine.

There is no requirement of dtbs_check for bisectability. Bindings are
separate (also exported to other users), thus it is expected to have
here async.

> 
> I'll consider devicetree validation as eventually valid from now on and 
> not care too much about it.

Everything will validate once reaches next as well...

Best regards,
Krzysztof
Krzysztof Kozlowski Feb. 13, 2023, 12:15 p.m. UTC | #4
On 13/02/2023 12:57, Janne Grunau wrote:
> On 2023-02-13 12:10:36 +0100, Krzysztof Kozlowski wrote:
>> On 12/02/2023 16:41, Janne Grunau wrote:
>>> The block on Apple M2 SoCs is compatible with the existing driver so
>>> just add its per-SoC compatible.
>>>
>>> Signed-off-by: Janne Grunau <j@jannau.net>
>>>
>>> ---
>>> This trivial dt-bindings update should be merged through the asahi-soc
>>> tree to ensure validation of the Apple M2 (t8112) devicetrees in this
>>> series.
>>
>> No, the bindings go via subsystem. Just because you want to validate
>> something is not really a reason - you can validate on next. Don't
>> create special rules for Asahi... or rather - why Asahi is special than
>> everyone else?
> 
> We did that 2 or 3 times in the past without commnts that it is not 
> desired so I wasn't aware that this would be special handling.
> 
> Merging binding and devicetree updates together looks to me like the 
> most sensible option since dtbs validation is the only testable 
> dependecy of dt binding updates.
> Keeping them together ensures the dtbs validate without delaying 
> devicetree changes by one kernel release after the dt-bindings change 
> was merged.
> I suppose it works out most of the time if the merge request is sent 
> only if it validates in next. That still depends on the merge order in 
> the merge window but -rc1 should be fine.

BTW, your approach causes also bisectability with checkpatch on the
drivers or the delay of drivers... Therefore not much solved here.

Best regards,
Krzysztof
Hector Martin Feb. 14, 2023, 2:35 a.m. UTC | #5
On 13/02/2023 21.10, Krzysztof Kozlowski wrote:
> On 13/02/2023 12:57, Janne Grunau wrote:
>> On 2023-02-13 12:10:36 +0100, Krzysztof Kozlowski wrote:
>>> On 12/02/2023 16:41, Janne Grunau wrote:
>>>> The block on Apple M2 SoCs is compatible with the existing driver so
>>>> just add its per-SoC compatible.
>>>>
>>>> Signed-off-by: Janne Grunau <j@jannau.net>
>>>>
>>>> ---
>>>> This trivial dt-bindings update should be merged through the asahi-soc
>>>> tree to ensure validation of the Apple M2 (t8112) devicetrees in this
>>>> series.
>>>
>>> No, the bindings go via subsystem. Just because you want to validate
>>> something is not really a reason - you can validate on next. Don't
>>> create special rules for Asahi... or rather - why Asahi is special than
>>> everyone else?
>>
>> We did that 2 or 3 times in the past without commnts that it is not 
>> desired so I wasn't aware that this would be special handling.
>>
>> Merging binding and devicetree updates together looks to me like the 
>> most sensible option since dtbs validation is the only testable 
>> dependecy of dt binding updates.
> 
> But it is not the recommended practice. Bindings were always going with
> drivers and this was said by Rob multiple times.
> 
> For sure if there is no driver update at all or subsystem maintainer is
> not responsive, bindings were picked up by SoC folks, but it's rather
> fallback, not the main path.

Rob also said that we can do trivial compatible additions ourselves and
don't have to involve him or subsystem maintainers. It's too trivial to
count as a "binding" change.

Let's not make this harder than it is. We have a pile of compatibles to
add every SoC that will only keep growing, and we have the situation
where this is largely a formality because it turns out the hardware *is*
compatible anyway (we just change the top compatible just in case). The
list of subsystems we touch will only keep growing. None of those
subsystem maintainers have any useful input to add to this, as the only
people with the information about what compatibles go together or don't
is us (since we're reverse engineering the hardware). All the relevant
bindings are listed in our section of MAINTAINERS. This stuff isn't
worth gratuitous added complexity and involvement. It's hard enough
getting driver changes into the kernel, let's not make it hard to get
devices that *don't* need driver changes on top of that.

>> I'll consider devicetree validation as eventually valid from now on and 
>> not care too much about it.
> 
> Everything will validate once reaches next as well...

Only once both changes hit next. If the DT change hits next first, it
won't validate. If the DT change hits mainline first, it won't validate.
AIUI from what Rob decently told me, this is okay, so we can submit
proper bindings changes to drivers from now on. But I still maintain
that *trivial compatible additions* should go through SoC because
there's no point in involving 15 subsystems instead of 1 every time
Apple releases a new SoC that's compatible with previous ones on 14
subsystems.

- Hector
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml
index 0dc957a56d35..673277a7a224 100644
--- a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml
+++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml
@@ -23,6 +23,7 @@  properties:
     items:
       - enum:
           - apple,t8103-pmgr
+          - apple,t8112-pmgr
           - apple,t6000-pmgr
       - const: apple,pmgr
       - const: syscon